Bug 1385013 - Check all vertexes for the target frame are outside of the parent frame if the target frame is empty. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Wed, 18 Oct 2017 10:43:22 +0900
changeset 682003 0f485764d78ec12059ebb8c3d4577f0e5989855c
parent 681956 f78d5947333422ab09ec23e3dab0d48538c9d6ad
child 736288 e41852d39d91b8c81e952c568842e4f52bb9aac7
push id84988
push userhikezoe@mozilla.com
push dateWed, 18 Oct 2017 01:43:53 +0000
reviewersbirtles
bugs1385013
milestone58.0a1
Bug 1385013 - Check all vertexes for the target frame are outside of the parent frame if the target frame is empty. r?birtles We create empty rectangle (zero-height or zero-width) frame for element which has no content inside it, e.g. '<p></p>'. And nsRect.Intersects returns false if either of the rectangles are empty, so if we check !transformedRect.Intersects(scrollableRect) and transformedRect is empty, we will end up returning true from IsFrameScrolledOutOfView even though the point represented by the empty transformedRect might be inside the scrollableRect. The reftest causes timeout without this fix since the animation never updates after the initial paint. MozReview-Commit-ID: FymFJfjxyGc
layout/generic/nsFrame.cpp
layout/reftests/css-animations/animation-on-empty-height-frame.html
layout/reftests/css-animations/reftest.list
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -10517,17 +10517,26 @@ IsFrameScrolledOutOfView(nsIFrame *aFram
   nsRect rect = aFrame->GetVisualOverflowRectRelativeToSelf();
 
   nsRect transformedRect =
     nsLayoutUtils::TransformFrameRectToAncestor(aFrame,
                                                 rect,
                                                 scrollableParent);
 
   nsRect scrollableRect = scrollableParent->GetVisualOverflowRect();
-  if (!transformedRect.Intersects(scrollableRect)) {
+  if (transformedRect.IsEmpty()) {
+    // If the transformed rect is empty it represents a line or a point that we
+    // should check is outside the the scrollable rect.
+    if (transformedRect.x > scrollableRect.XMost() ||
+        transformedRect.y > scrollableRect.YMost() ||
+        scrollableRect.x > transformedRect.XMost() ||
+        scrollableRect.y > transformedRect.YMost()) {
+      return true;
+    }
+  } else if (!transformedRect.Intersects(scrollableRect)) {
     return true;
   }
 
   nsIFrame* parent = scrollableParent->GetParent();
   if (!parent) {
     return false;
   }
 
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-animations/animation-on-empty-height-frame.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html class="reftest-wait reftest-no-flush">
+<style>
+@keyframes anim {
+  from { background-color: white; }
+  to { background-color: red; }
+}
+</style>
+<body>
+</body>
+<script>
+window.addEventListener('load', () => {
+  const body = document.querySelector('body');
+  body.style.animation = 'anim 100s step-end reverse';
+  body.addEventListener('animationstart', () => {
+    // This MozAfterPaint event corresponds to the white background paint.
+    // (The animation will initially paint the background red since it is playing
+    // a step-end animation in reverse.)
+    window.addEventListener('MozAfterPaint', () => {
+      document.documentElement.classList.remove('reftest-wait');
+    }, {once: true});
+  });
+});
+</script>
+</html>
--- a/layout/reftests/css-animations/reftest.list
+++ b/layout/reftests/css-animations/reftest.list
@@ -1,14 +1,15 @@
 == screen-animations.html screen-animations-ref.html
 != screen-animations.html screen-animations-notref.html
 skip-if(stylo) fails == print-no-animations.html print-no-animations-ref.html # reftest harness doesn't actually make pres context non-dynamic for reftest-paged tests. Bug 1374154 for stylo
 skip-if(stylo) fails != print-no-animations.html print-no-animations-notref.html # reftest harness doesn't actually make pres context non-dynamic for reftest-paged tests. Bug 1374154 for stylo
 == animate-opacity.html animate-opacity-ref.html
 == animate-preserves3d.html animate-preserves3d-ref.html
+== animation-on-empty-height-frame.html about:blank
 == in-visibility-hidden-animation.html in-visibility-hidden-animation-ref.html
 == in-visibility-hidden-animation-pseudo-element.html in-visibility-hidden-animation-pseudo-element-ref.html
 == partially-out-of-view-animation.html partially-out-of-view-animation-ref.html
 == animate-display-table-opacity.html animate-display-table-opacity-ref.html
 # We need to run 100% opacity test case when OMTA is disabled to check that the animation creates a stacking context even if the animation is not running on the compositor
 test-pref(layers.offmainthreadcomposition.async-animations,false) == stacking-context-opacity-1-animation.html stacking-context-animation-ref.html
 # We need to run transform:none test case when OMTA is disabled to check that the animation creates a stacking context even if the animation is not running on the compositor
 test-pref(layers.offmainthreadcomposition.async-animations,false) == stacking-context-transform-none-animation.html stacking-context-animation-ref.html