Commit 6aa7507e authored by Emilio Cobos Álvarez's avatar Emilio Cobos Álvarez
Browse files

Bug 1543599 - Don't suppress scroll anchoring adjustments when switching... 

Bug 1543599 - Don't suppress scroll anchoring adjustments when switching display to `none`. r=dholbert

Since that means that we won't suppress them when switching display back (since
we have no frame to pull the old style from).

We may want to match Chrome more exactly and don't do this any time `display`
changes (which if I'm reading their code correctly is what they do...).

But for now I've done the minimal thing and added a test.

Differential Revision: https://phabricator.services.mozilla.com/D38094

--HG--
extra : moz-landing-system : lando
parent 6ef01a0d

layout/base/RestyleManager.cpp

+18 −5
Original line number Diff line number Diff line
@@ -2747,12 +2747,25 @@ bool RestyleManager::ProcessPostTraversal(Element* aElement, 
  // XXXbholley: We should teach the frame constructor how to clear the dirty

  // descendants bit to avoid the traversal here.

  if (changeHint & nsChangeHint_ReconstructFrame) {

    if (wasRestyled && styleFrame &&

        styleFrame->StyleDisplay()->IsAbsolutelyPositionedStyle() !=

            upToDateStyleIfRestyled->StyleDisplay()

                ->IsAbsolutelyPositionedStyle()) {

    if (wasRestyled && styleFrame) {

      auto* oldDisp = styleFrame->StyleDisplay();

      auto* newDisp = upToDateStyleIfRestyled->StyleDisplay();

      // https://drafts.csswg.org/css-scroll-anchoring/#suppression-triggers

      //

      // We need to do the position check here rather than in

      // DidSetComputedStyle because changing position reframes.

      //

      // Don't suppress adjustments when going back to display: none, regardless

      // of whether we're abspos changes.

      //

      // TODO(emilio): I _think_ chrome won't suppress adjustments whenever

      // `display` changes. But ICBW.

      if (newDisp->mDisplay != StyleDisplay::None &&

          oldDisp->IsAbsolutelyPositionedStyle() !=

              newDisp->IsAbsolutelyPositionedStyle()) {

        aRestyleState.AddPendingScrollAnchorSuppression(styleFrame);

      }

    }

    ClearRestyleStateFromSubtree(aElement);

    return true;

  }

testing/web-platform/tests/css/css-scroll-anchoring/position-change-heuristic-display-none-change.html

0 → 100644
+72 −0
Original line number Diff line number Diff line 
<!DOCTYPE html>

<script src="/resources/testharness.js"></script>

<script src="/resources/testharnessreport.js"></script>

<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">

<link rel="author" title="Mozilla" href="https://mozilla.org">

<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1543599">

<link rel="help" href="https://drafts.csswg.org/css-scroll-anchoring/#suppression-triggers">

<style>

#hidden {

  display: none;

  background: red;

}

#spacer {

  height: calc(100vh + 200px); /* At least 200px of scroll range */

}

</style>

<table>

  <thead>

    <tr>

      <th>1

      <th>1

      <th>1

      <th>1

    </tr>

  </thead>

  <thead id="hidden">

    <tr>

      <th>1

      <th>1

      <th>1

      <th>1

    </tr>

  </thead>

  <tbody>

    <tr><td>0 <td>0 <td>0 <td>0 </tr>

    <tr><td>0 <td>0 <td>0 <td>0 </tr>

    <tr><td>0 <td>0 <td>0 <td>0 </tr>

    <tr><td>0 <td>0 <td>0 <td>0 </tr>

    <tr><td>0 <td>0 <td>0 <td>0 </tr>

    <tr><td>0 <td>0 <td>0 <td>0 </tr>

    <tr><td>0 <td>0 <td>0 <td>0 </tr>

  </tbody>

</table>

<div id="spacer"></div>

<script>

let firstEvent = true;

const targetScrollPosition = 100;

const hidden = document.querySelector("#hidden");

const t = async_test("Scroll offset doesn't change when an element goes back and forth to display: none");

window.onscroll = t.step_func(function() {

  hidden.style.display = "block";

  hidden.style.position = "absolute";

  hidden.style.visibility = "hidden";

  window.unused = hidden.offsetHeight;

  hidden.style.display = "";

  hidden.style.position = "";

  hidden.style.visibility = "";



  if (firstEvent) {

    firstEvent = false;

    requestAnimationFrame(t.step_func(function() {

      requestAnimationFrame(t.step_func_done(function() {

        assert_equals(document.scrollingElement.scrollTop, targetScrollPosition);

      }));

    }));

  }

});



window.onload = t.step_func(function() {

  window.scrollTo(0, targetScrollPosition);

});

</script>