Bug 1498873 - Pass the right frame to PushAbsoluteContainingBlock to determine whether we're a fixed-pos containing block. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 17 Oct 2018 20:22:38 +0000
changeset 500275 b77bde54527692f87c31a60112d3cb57ec13298e
parent 500274 7f9d03c29a6ffd82c1b5e17c14e27a2ae9d64434
child 500276 1249680014b0b4faf9eed339f9716755322bb33d
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1498873
milestone64.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 1498873 - Pass the right frame to PushAbsoluteContainingBlock to determine whether we're a fixed-pos containing block. r=bzbarsky When we're creating a scrollframe with let's say, display: flex or grid, the containing block is the grid container itself, but the transformed frame is the scroll frame. This is the only caller that (incorrectly) passes the same frame to PushAbsoluteContainingBlock. Our painting code deals with it, mostly, because it starts from the placeholder to paint fixed items, and it hits the scrollframe, but it gets confused sometimes causing the issue described here. I'll find a way to add a crashtest for this, and maybe a reftest, though this works in non-WR. We should probably add a few more assertions to the frame constructor... Differential Revision: https://phabricator.services.mozilla.com/D8724
layout/base/nsCSSFrameConstructor.cpp
testing/web-platform/tests/css/css-transforms/dynamic-fixed-pos-cb-change-ref.html
testing/web-platform/tests/css/css-transforms/dynamic-fixed-pos-cb-change.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -3922,19 +3922,18 @@ nsCSSFrameConstructor::ConstructFrameFro
         // except without the assertion that the style display and frame match.
         // When constructing scroll frames we intentionally use the style
         // display for the outer, but make the inner the containing block.
         if ((maybeAbsoluteContainingBlockDisplay->IsAbsolutelyPositionedStyle() ||
              maybeAbsoluteContainingBlockDisplay->IsRelativelyPositionedStyle() ||
              maybeAbsoluteContainingBlockDisplay->IsFixedPosContainingBlock(
                  maybeAbsoluteContainingBlockStyleFrame)) &&
             !nsSVGUtils::IsInSVGTextSubtree(maybeAbsoluteContainingBlockStyleFrame)) {
-          nsContainerFrame* cf = static_cast<nsContainerFrame*>(
-              maybeAbsoluteContainingBlock);
-          aState.PushAbsoluteContainingBlock(cf, cf, absoluteSaveState);
+          auto* cf = static_cast<nsContainerFrame*>(maybeAbsoluteContainingBlock);
+          aState.PushAbsoluteContainingBlock(cf, maybeAbsoluteContainingBlockStyleFrame, absoluteSaveState);
         }
       }
 
       if (bits & FCDATA_USE_CHILD_ITEMS) {
         nsFrameConstructorSaveState floatSaveState;
 
         if (ShouldSuppressFloatingOfDescendants(newFrame)) {
           aState.PushFloatContainingBlock(nullptr, floatSaveState);
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-transforms/dynamic-fixed-pos-cb-change-ref.html
@@ -0,0 +1,47 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<meta charset="utf-8">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<style>
+html,
+body {
+  height: 100%;
+  overflow: hidden;
+  margin: 0;
+  padding: 0;
+  background-color: #222;
+}
+
+body {
+  display: flex;
+  transform: scale(0.9);
+}
+
+#fixed {
+  position: fixed;
+  width: 100%;
+  height: 100%;
+  top: 0;
+  left: 0;
+  z-index: 1;
+  background: black;
+}
+
+#popup {
+  position: absolute;
+  width: 200px;
+  height: 200px;
+  left: 0;
+  top: 0;
+  z-index: 2;
+  background: green;
+  transform: scale( 1.1111 );
+}
+
+body, #popup {
+  transform-origin: 50% 50%;
+}
+</style>
+<div id="popup"></div>
+<div id="fixed"></div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-transforms/dynamic-fixed-pos-cb-change.html
@@ -0,0 +1,65 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>CSS Test: Checks that dynamic changes to whether we're a fixed-pos containing block are handled correctly</title>
+<meta charset="utf-8">
+<link rel="help" href="https://drafts.csswg.org/css-transforms-1/#containing-block-for-all-descendants">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1498873">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<link rel="match" href="dynamic-fixed-pos-cb-change-ref.html">
+<style>
+html,
+body {
+  height: 100%;
+  overflow: hidden;
+  margin: 0;
+  padding: 0;
+  background-color: #222;
+}
+
+body {
+  display: flex;
+}
+
+#fixed {
+  position: fixed;
+  width: 100%;
+  height: 100%;
+  top: 0;
+  left: 0;
+  z-index: 1;
+  background: black;
+}
+
+#popup {
+  position: absolute;
+  width: 200px;
+  height: 200px;
+  left: 0;
+  top: 0;
+  z-index: 2;
+  background: red;
+  transform: scale( 1.1111 );
+}
+
+body, #popup {
+  transform-origin: 50% 50%;
+}
+</style>
+<div id="popup">
+  <!-- Seeing this box change color should not move me or change my size. -->
+</div>
+<div id="fixed"></div>
+<script>
+onload = function() {
+  requestAnimationFrame(() => {
+    document.body.style.transform = "scale(0.9)";
+    requestAnimationFrame(() => {
+      popup.style.backgroundColor = "green";
+      requestAnimationFrame(() => {
+        document.documentElement.className = "";
+      });
+    });
+  });
+}
+</script>