Bug 1585882 - Fix needs_frame() check to account for the case where a…

…n ancestor of us has been reconstructed regularly, not via lazy frame construction. r=heycam This is a pre-existing bug, and this would be enough to fix the website, but this is still not 100% correct. More on that in a second. Differential Revision: https://phabricator.services.mozilla.com/D48134
mozilla-spidermonkey · Oct 5, 2019 · 6cca6c0 · 6cca6c0
1 parent f91d6b4
commit 6cca6c0
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 1 deletion.
diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs
@@ -6137,6 +6137,7 @@ pub extern "C" fn Servo_ProcessInvalidations(
 
 #[no_mangle]
 pub extern "C" fn Servo_HasPendingRestyleAncestor(element: &RawGeckoElement) -> bool {
+    let mut has_yet_to_be_styled = false;
     let mut element = Some(GeckoElement(element));
     while let Some(e) = element {
         if e.has_animations() {
@@ -6146,15 +6147,23 @@ pub extern "C" fn Servo_HasPendingRestyleAncestor(element: &RawGeckoElement) ->
         // If the element needs a frame, it means that we haven't styled it yet
         // after it got inserted in the document, and thus we may need to do
         // that for transitions and animations to trigger.
+        //
+        // This is a fast path in the common case, but `has_yet_to_be_styled` is
+        // the real check for this.
         if e.needs_frame() {
             return true;
         }
 
-        if let Some(data) = e.borrow_data() {
+        let data = e.borrow_data();
+        if let Some(ref data) = data {
             if !data.hint.is_empty() {
                 return true;
             }
+            if has_yet_to_be_styled && !data.styles.is_display_none() {
+                return true;
+            }
         }
+        has_yet_to_be_styled = data.is_none();
 
         element = e.traversal_parent();
     }

diff --git a/testing/web-platform/tests/css/cssom/getComputedStyle-animations-replaced-into-ib-split.html b/testing/web-platform/tests/css/cssom/getComputedStyle-animations-replaced-into-ib-split.html
@@ -0,0 +1,34 @@
+<!doctype html>
+<title>getComputedStyle() returns the right style for animating nodes that have been just inserted into the document, and that have an ancestor whose layout tree was recreated (like an IB-split)</title>
+<link rel="help" href="https://drafts.csswg.org/cssom/#dom-window-getcomputedstyle">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1585882">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<style>
+  @keyframes my-animation {
+    from { color: green; }
+    to { color: green; }
+  }
+  div {
+    color: red;
+    animation: my-animation 1s infinite linear paused;
+  }
+</style>
+<span>
+  <div></div>
+</span>
+<script>
+test(() => {
+  let oldDiv = document.querySelector("div");
+  window.unused = oldDiv.getBoundingClientRect(); // update layout
+
+  assert_equals(getComputedStyle(oldDiv).color, "rgb(0, 128, 0)", "Should take color from the animation");
+
+  let newDiv = document.createElement("div");
+  oldDiv.replaceWith(newDiv);
+
+  assert_equals(getComputedStyle(newDiv).color, "rgb(0, 128, 0)", "Should take color from the animation (just inserted into the document)");
+}, "getComputedStyle() should return animation styles for nodes just inserted into the document, even if they're in an IB-split");
+</script>
diff --git a/...eb-platform/tests/css/cssom/getComputedStyle-layout-dependent-replaced-into-ib-split.html b/...eb-platform/tests/css/cssom/getComputedStyle-layout-dependent-replaced-into-ib-split.html
@@ -0,0 +1,30 @@
+<!doctype html>
+<title>getComputedStyle() returns the right style for layout-dependent properties for nodes that have been just inserted into the document, and that have an ancestor whose layout tree was recreated (like an IB-split)</title>
+<link rel="help" href="https://drafts.csswg.org/cssom/#dom-window-getcomputedstyle">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1585882">
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="author" title="Mozilla" href="https://mozilla.org">
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<style>
+  div {
+    width: 100%;
+  }
+</style>
+<span>
+  <div></div>
+</span>
+<script>
+test(() => {
+  let oldDiv = document.querySelector("div");
+  window.unused = oldDiv.getBoundingClientRect(); // update layout
+
+  let oldWidth = getComputedStyle(oldDiv).width;
+  assert_true(oldWidth.indexOf("px") !== -1, "Should return the used value for width");
+
+  let newDiv = document.createElement("div");
+  oldDiv.replaceWith(newDiv);
+
+  assert_equals(getComputedStyle(newDiv).width, oldWidth, "Should return the used value for width (just inserted into the document)");
+}, "getComputedStyle() should return used value correctly for nodes just inserted into the document, even if they're in an IB-split");
+</script>