REGRESSION (r187121): Can't get to the main content of the page at ht…

…tps://theintercept.com/drone-papers/ https://bugs.webkit.org/show_bug.cgi?id=151849 rdar://problem/23132828 Reviewed by Zalan Bujtas. Source/WebCore: This page uses a fill-forwards animation where the last keyframe has height: auto. After r187121, we tried to blend the height Length value from the last keyframe to the first keyframe with progress=0 (which should pick up the 'auto' from the last keyframe). However, Length::blend() just considered both 0 and 'auto' to be zero, and returned the 0 length. So fix Length::blend() to return the "from" length if progress is zero. Test: animations/fill-forwards-auto-height.html * page/animation/CSSPropertyAnimation.cpp: (WebCore::blendFunc): Length::blend takes a double, so don't narrow to float. * page/animation/KeyframeAnimation.cpp: (WebCore::KeyframeAnimation::fetchIntervalEndpointsForProperty): Declare two variables at first use. * platform/Length.h: (WebCore::Length::blend): LayoutTests: New ref test. The behavior of imported/blink/transitions/transition-not-interpolable.html changed with this patch, but that test is trying to determine if transitions run to/from 'auto' values, and doing it wrong. The current patch doesn't change the user-visible behavior of transitions with 'auto' endpoints (covered by http://webkit.org/b/38243). * animations/fill-forwards-auto-height-expected.html: Added. * animations/fill-forwards-auto-height.html: Added. * imported/blink/transitions/transition-not-interpolable-expected.txt: Canonical link: https://commits.webkit.org/170060@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@193610 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Dec 7, 2015 · 3e271eb249c2d0c0336a7dc078cf837d8edb36fb · 3e271eb
1 parent 6b373c3
commit 3e271eb249c2d0c0336a7dc078cf837d8edb36fb
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,22 @@
+2015-12-06  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r187121): Can't get to the main content of the page at https://theintercept.com/drone-papers/
+        https://bugs.webkit.org/show_bug.cgi?id=151849
+        rdar://problem/23132828
+
+        Reviewed by Zalan Bujtas.
+
+        New ref test.
+
+        The behavior of imported/blink/transitions/transition-not-interpolable.html changed
+        with this patch, but that test is trying to determine if transitions run to/from
+        'auto' values, and doing it wrong. The current patch doesn't change the user-visible
+        behavior of transitions with 'auto' endpoints (covered by http://webkit.org/b/38243).
+
+        * animations/fill-forwards-auto-height-expected.html: Added.
+        * animations/fill-forwards-auto-height.html: Added.
+        * imported/blink/transitions/transition-not-interpolable-expected.txt:
+
 2015-12-06  David Kilzer  <ddkilzer@apple.com>
 
         REGRESSION(r193584): Causes heap use-after-free crashes in Web Inspector tests with AddressSanitizer (Requested by ddkilzer on #webkit).

diff --git a/LayoutTests/animations/fill-forwards-auto-height-expected.html b/LayoutTests/animations/fill-forwards-auto-height-expected.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    .landing {
+        width: 300px;
+        overflow: hidden;
+        border: 1px solid black;
+    }
+
+    .contents {
+        height: 300px;
+        background-color: gray;
+    }
+    </style>
+</head>
+<body>
+
+<div class="landing">
+    <div class="contents">
+
+    </div>
+</div>
+</body>
+</html>
diff --git a/LayoutTests/animations/fill-forwards-auto-height.html b/LayoutTests/animations/fill-forwards-auto-height.html
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+    @keyframes reveal {
+        0%, 50% {
+            height: 0;
+        }
+
+        100% {
+            height: auto;
+        }
+    }
+
+    .landing {
+        width: 300px;
+        overflow: hidden;
+        border: 1px solid black;
+        animation: reveal 0.01s forwards;
+    }
+
+    .contents {
+        height: 300px;
+        background-color: gray;
+    }
+    </style>
+</head>
+<body>
+<div id="animated" class="landing">
+    <div class="contents"></div>
+</div>
+<script>
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    document.getElementById('animated').addEventListener('animationend', function() {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    })
+</script>
+</body>
+</html>
diff --git a/LayoutTests/imported/blink/transitions/transition-not-interpolable-expected.txt b/LayoutTests/imported/blink/transitions/transition-not-interpolable-expected.txt
@@ -1 +1 @@
-PASS
+FAIL -- transtion should not apply from 0px to auto
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2015-12-06  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r187121): Can't get to the main content of the page at https://theintercept.com/drone-papers/
+        https://bugs.webkit.org/show_bug.cgi?id=151849
+        rdar://problem/23132828
+
+        Reviewed by Zalan Bujtas.
+
+        This page uses a fill-forwards animation where the last keyframe has height: auto.
+        After r187121, we tried to blend the height Length value from the last keyframe to the
+        first keyframe with progress=0 (which should pick up the 'auto' from the last keyframe).
+
+        However, Length::blend() just considered both 0 and 'auto' to be zero, and returned
+        the 0 length.
+
+        So fix Length::blend() to return the "from" length if progress is zero.
+
+        Test: animations/fill-forwards-auto-height.html
+
+        * page/animation/CSSPropertyAnimation.cpp:
+        (WebCore::blendFunc): Length::blend takes a double, so don't narrow to float.
+        * page/animation/KeyframeAnimation.cpp:
+        (WebCore::KeyframeAnimation::fetchIntervalEndpointsForProperty): Declare two variables
+        at first use.
+        * platform/Length.h:
+        (WebCore::Length::blend):
+
 2015-12-06  Simon Fraser  <simon.fraser@apple.com>
 
         Give SVGTransformList some inline vector capacity

diff --git a/Source/WebCore/page/animation/CSSPropertyAnimation.cpp b/Source/WebCore/page/animation/CSSPropertyAnimation.cpp
@@ -81,7 +81,7 @@ static inline Color blendFunc(const AnimationBase*, const Color& from, const Col
 
 static inline Length blendFunc(const AnimationBase*, const Length& from, const Length& to, double progress)
 {
-    return to.blend(from, narrowPrecisionToFloat(progress));
+    return to.blend(from, progress);
 }
 
 static inline LengthSize blendFunc(const AnimationBase* anim, const LengthSize& from, const LengthSize& to, double progress)
@@ -256,6 +256,7 @@ static inline SVGLength blendFunc(const AnimationBase*, const SVGLength& from, c
 {
     return to.blend(from, narrowPrecisionToFloat(progress));
 }
+
 static inline Vector<SVGLength> blendFunc(const AnimationBase*, const Vector<SVGLength>& from, const Vector<SVGLength>& to, double progress)
 {
     size_t fromLength = from.size();
@@ -1469,7 +1470,6 @@ bool CSSPropertyAnimation::blendProperties(const AnimationBase* anim, CSSPropert
         wrapper->blend(anim, dst, a, b, progress);
         return !wrapper->animationIsAccelerated() || !anim->isAccelerated();
     }
-
     return false;
 }
 

diff --git a/Source/WebCore/page/animation/KeyframeAnimation.cpp b/Source/WebCore/page/animation/KeyframeAnimation.cpp
@@ -100,9 +100,6 @@ void KeyframeAnimation::fetchIntervalEndpointsForProperty(CSSPropertyID property
         prevIndex = i;
     }
 
-    double scale = 1;
-    double offset = 0;
-
     if (prevIndex == -1)
         prevIndex = 0;
 
@@ -122,8 +119,8 @@ void KeyframeAnimation::fetchIntervalEndpointsForProperty(CSSPropertyID property
     fromStyle = prevKeyframe.style();
     toStyle = nextKeyframe.style();
 
-    offset = prevKeyframe.key();
-    scale = 1.0 / (nextKeyframe.key() - prevKeyframe.key());
+    double offset = prevKeyframe.key();
+    double scale = 1.0 / (nextKeyframe.key() - prevKeyframe.key());
 
     prog = progress(scale, offset, prevKeyframe.timingFunction(name()));
 }

diff --git a/Source/WebCore/platform/Length.h b/Source/WebCore/platform/Length.h
@@ -422,7 +422,7 @@ inline Length Length::blend(const Length& from, double progress) const
         return blendMixedTypes(from, progress);
 
     if (from.isZero() && isZero())
-        return *this;
+        return progress ? *this : from; // Pick up 'auto' from 'from' if progress is zero.
 
     LengthType resultType = type();
     if (isZero())