Bug 1621725 [wpt PR 22195] - Fix position:sticky when inside fixed subtree, a=testonly
authorDavid Bokan <bokan@chromium.org>
Sat, 14 Mar 2020 11:30:23 +0000
changeset 518864 b7893b69cea5c05c66bb50d21ac0001e1e87b719
parent 518863 c1ba1ca6ecd38b94c49c9c6304b4584943fa9406
child 518865 1520600a583881d5300bc8425e7ce472602a8686
push id37217
push userccoroiu@mozilla.com
push dateSun, 15 Mar 2020 21:37:59 +0000
treeherdermozilla-central@f9fc9427476e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1621725, 22195, 1019142, 2097542, 750264
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1621725 [wpt PR 22195] - Fix position:sticky when inside fixed subtree, a=testonly Automatic update from web-platform-tests Fix position:sticky when inside fixed subtree In https://crrev.com/4d25b125dae5 I changed the scroll tree parenting logic so that elements in position:fixed subtrees have the LayoutView's ScrollNode as the scroll parent. This made sense since scrolling over a fixed element should cause the document to scroll. However, this is slightly different from how the transform tree looks. Because scrolling the document doesn't cause position:fixed eement so translate, these nodes don't have the LayoutView's ScrollTranslation transform node as an ancestor. As a simple example, a scrolling document with a position:fixed <div> scroller will generate the following scroll and transform trees (approximately): *ScrollTree* *TransformTree* Root Root | | VisualViewport Translation VisualViewport | / \ LayoutView Translation / \ | Fixed LayoutView Fixed Scroller Translation The situation above makes sense for what parent-child relationships mean in each tree: the scroll tree encodes how scrolls chain; scrolling on a child should bubble up to its parent in this tree. The transform tree encodes the physical effect of scrolling a node. In the above example, scrolling from the fixed scroller should bubble up to the LayoutView (when the scroller is fully scrolled) but scrolling the LayoutView will not cause movement of the fixed scroller. The above makes sense but caused sticky code to get confused. A sticky constraint is attached to the scroll translation node. With the above situation, this meant that inside a fixed subtree, we'd attach it to the VisualViewport's scroll translation node. This was unexpected; the constraints are in "document coordinates", meaning that to translate them into the viewport space we must apply the scroll offset [1]. The compositor would use the visual viewport's (typically 0) scroll offset to adjust these values, leading to incorrect calculations. This previously worked because the scroll node used in a fixed subtree would be the visual viewport (before the CL mentioned at the top). In [2] we check whether the current overflow clip is also our scroller, prior to the CL this check have failed because "our scroller" was the visual viewport but our clip was the LayoutView. Now they are both the LayoutView. The fix in this CL is to make the check in [2] more stringent; we also want to make sure that our scroller is the nearest scroller in the transform tree. That is, if we scroll it, will we cause the current node to move? If not, we don't need a sticky constraint on the compositor because user scrolling can't change the sticky's offset relative to its clip. [1] https://cs.chromium.org/chromium/src/cc/trees/property_tree.cc?l=321&rcl=628f869d1fda631a85f051ad13b5d278319298fc [2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc?l=553&rcl=99a5a1266f303ba6ae46174a2b4cbd165ea7e934 Bug: 1019142 Change-Id: I781943ff43514905d399803c780c6081d7d47e8f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097542 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#750264} -- wpt-commits: 1e6645ce41bc902a4c60d6f90a934f9d6d8a258a wpt-pr: 22195
testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor-iframe-ref.html
testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor-iframe.html
testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor-ref.html
testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor.html
testing/web-platform/tests/css/css-position/resources/position-sticky-fixed-ancestor-iframe-child.html
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor-iframe-ref.html
@@ -0,0 +1,93 @@
+<!DOCTYPE html>
+<title>Sticky elements inside fixed ancestors and iframe shouldn't account for scroll</title>
+
+<style>
+  body,html {
+    margin: 0;
+    width: 100%;
+    height: 100%;
+  }
+
+  iframe {
+    margin: 10px;
+    width: 90%;
+    height: 90%;
+    border: 1px solid black;
+  }
+
+  .spacer {
+    height: 120vh;
+  }
+</style>
+
+<div class="spacer"></div>
+<iframe srcdoc="
+  <!DOCTYPE html>
+  <title>Reference for sticky elements inside fixed ancestors shouldn't account for scroll</title>
+  <style>
+    body,html {
+      margin: 0;
+      width: 100%;
+      height: 100%;
+    }
+
+    .sticky {
+      background: green;
+      width: 100px;
+      height: 10%;
+    }
+
+    .spacer {
+      height: calc(25vh - 10%);
+    }
+    .long {
+      height: 600vh;
+    }
+
+    .position-parent {
+      position: absolute;
+      display: flex;
+      align-items: center;
+      justify-content: center;
+      width: 100%;
+      height: 100%;
+      top: 100vh;
+      background-color: lightgrey;
+    }
+
+    .container {
+      width: 100px;
+      height: 100%;
+      background-color: grey;
+    }
+
+    button {
+      position: fixed;
+      left: 20px;
+      top: 20px;
+    }
+
+    .fixed {
+      position: fixed;
+      top: 25vh;
+    }
+  </style>
+
+  <div class='position-parent fixed'>
+    <div class='container'>
+      <div class='spacer'></div>
+      <div class='sticky'></div>
+    </div>
+  </div>
+  <div class='long'></div>
+  <button id='button'>Toggle Fixed</button>
+  <script>
+    window.scrollTo(0, document.querySelector('.long').clientHeight);
+  </script>
+"></iframe>
+<div class="spacer"></div>
+
+<script>
+  const child = document.querySelector('iframe');
+  child.scrollIntoView();
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor-iframe.html
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html class='reftest-wait'>
+<title>Sticky elements inside fixed ancestors and iframe shouldn't account for scroll</title>
+<link rel="match" href="position-sticky-fixed-ancestor-iframe-ref.html" />
+<link rel="help" href="https://www.w3.org/TR/css-position-3/#sticky-pos" />
+<link rel="help" href="https://crbug.com/1019142">
+<meta name="assert" content="This test checks that a sticky element inside a fixed subtree and iframe doesn't scroll with the viewport "/>
+
+<script src="/common/reftest-wait.js"></script>
+
+<style>
+  body,html {
+    margin: 0;
+    width: 100%;
+    height: 100%;
+  }
+
+  iframe {
+    margin: 10px;
+    width: 90%;
+    height: 90%;
+    border: 1px solid black;
+  }
+
+  .spacer {
+    height: 120vh;
+  }
+</style>
+
+<div class="spacer"></div>
+<iframe src="resources/position-sticky-fixed-ancestor-iframe-child.html"></iframe>
+<div class="spacer"></div>
+
+<script>
+  const child = document.querySelector('iframe');
+  child.scrollIntoView();
+  window.onload = () => {
+    const childDoc = child.contentDocument;
+    function toggleFixed() {
+      childDoc.querySelector('.position-parent').classList.toggle('fixed');
+    }
+
+    childDoc.getElementById('button').addEventListener('click', toggleFixed);
+
+    requestAnimationFrame(() => {
+      childDoc.scrollingElement.scrollTop = childDoc.querySelector('.long').clientHeight;
+      requestAnimationFrame(() => {
+        toggleFixed();
+        takeScreenshot();
+      });
+    });
+  };
+</script>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor-ref.html
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<title>Reference for sticky elements inside fixed ancestors shouldn't account for scroll</title>
+<style>
+  body,html {
+    margin: 0;
+    width: 100%;
+    height: 100%;
+  }
+
+  .sticky {
+    background: green;
+    width: 100px;
+    height: 10%;
+  }
+
+  .spacer {
+    height: calc(25vh - 10%);
+  }
+  .long {
+    height: 600vh;
+  }
+
+  .position-parent {
+    position: absolute;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    width: 100%;
+    height: 100%;
+    top: 100vh;
+    background-color: lightgrey;
+  }
+
+  .container {
+    width: 100px;
+    height: 100%;
+    background-color: grey;
+  }
+
+  button {
+    position: fixed;
+    left: 20px;
+    top: 20px;
+  }
+
+  .fixed {
+    position: fixed;
+    top: 25vh;
+  }
+</style>
+
+<div class="position-parent fixed">
+  <div class="container">
+    <div class="spacer"></div>
+    <div class="sticky"></div>
+  </div>
+</div>
+<div class="long"></div>
+<button id="button">Toggle Fixed</button>
+
+<script>
+  window.scrollTo(0, document.querySelector('.long').clientHeight);
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-position/position-sticky-fixed-ancestor.html
@@ -0,0 +1,86 @@
+<!DOCTYPE html>
+<html class='reftest-wait'>
+<title>Sticky elements inside fixed ancestors shouldn't account for scroll</title>
+<link rel="match" href="position-sticky-fixed-ancestor-ref.html" />
+<link rel="help" href="https://www.w3.org/TR/css-position-3/#sticky-pos" />
+<link rel="help" href="https://crbug.com/1019142">
+<meta name="assert" content="This test checks that a sticky element inside a fixed subtree doesn't scroll with the viewport "/>
+
+<script src="/common/reftest-wait.js"></script>
+
+<style>
+  body,html {
+    margin: 0;
+    width: 100%;
+    height: 100%;
+  }
+
+  .sticky {
+    background: green;
+    position: sticky;
+    bottom: 50vh;
+    width: 100px;
+    height: 10%;
+  }
+
+  .spacer {
+    height: 90%;
+  }
+  .long {
+    height: 600vh;
+  }
+
+  .position-parent {
+    position: absolute;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    width: 100%;
+    height: 100%;
+    top: 100vh;
+    background-color: lightgrey;
+  }
+
+  .container {
+    width: 100px;
+    height: 100%;
+    background-color: grey;
+  }
+
+  button {
+    position: fixed;
+    left: 20px;
+    top: 20px;
+  }
+
+  .fixed {
+    position: fixed;
+    top: 25vh;
+  }
+</style>
+
+<div class="position-parent">
+  <div class="container">
+    <div class="spacer"></div>
+    <div class="sticky"></div>
+  </div>
+</div>
+<div class="long"></div>
+<button id="button">Toggle Fixed</button>
+
+<script>
+  function toggleFixed() {
+    document.querySelector('.position-parent').classList.toggle('fixed');
+  }
+
+  document.getElementById('button').addEventListener('click', toggleFixed);
+
+  requestAnimationFrame(() => {
+    window.scrollTo(0, document.querySelector('.long').clientHeight);
+    requestAnimationFrame(() => {
+      toggleFixed();
+      takeScreenshot();
+    });
+  });
+</script>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-position/resources/position-sticky-fixed-ancestor-iframe-child.html
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<style>
+  body,html {
+    margin: 0;
+    width: 100%;
+    height: 100%;
+  }
+
+  .sticky {
+    background: green;
+    position: sticky;
+    bottom: 50vh;
+    width: 100px;
+    height: 10%;
+  }
+
+  .spacer {
+    height: 90%;
+  }
+  .long {
+    height: 600vh;
+  }
+
+  .position-parent {
+    position: absolute;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    width: 100%;
+    height: 100%;
+    top: 100vh;
+    background-color: lightgrey;
+  }
+
+  .container {
+    width: 100px;
+    height: 100%;
+    background-color: grey;
+  }
+
+  button {
+    position: fixed;
+    left: 20px;
+    top: 20px;
+  }
+
+  .fixed {
+    position: fixed;
+    top: 25vh;
+  }
+</style>
+
+<div class="position-parent">
+  <div class="container">
+    <div class="spacer"></div>
+    <div class="sticky"></div>
+  </div>
+</div>
+<div class="long"></div>
+<button id="button">Toggle Fixed</button>
+</html>