Changeset 193610 in webkit

Timestamp:

Dec 6, 2015 9:14:30 PM (8 years ago)

Author:

Simon Fraser

Message:

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.

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/animations/fill-forwards-auto-height-expected.html (added)
• LayoutTests/animations/fill-forwards-auto-height.html (added)
• LayoutTests/imported/blink/transitions/transition-not-interpolable-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/animation/CSSPropertyAnimation.cpp (modified) (3 diffs)
• Source/WebCore/page/animation/KeyframeAnimation.cpp (modified) (2 diffs)
• Source/WebCore/platform/Length.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/imported/blink/transitions/transition-not-interpolable-expected.txt

@@ -1 @@
-PASS
+FAIL -- transtion should not apply from 0px to auto

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp

@@ -82 +82 @@
 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);
 }
 
@@ -257 +257 @@
     return to.blend(from, narrowPrecisionToFloat(progress));
 }
+
 static inline Vector<SVGLength> blendFunc(const AnimationBase*, const Vector<SVGLength>& from, const Vector<SVGLength>& to, double progress)
 {
@@ -1470 +1471 @@
         return !wrapper->animationIsAccelerated() || !anim->isAccelerated();
     }
-
     return false;
 }

trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp

@@ -101 +101 @@
     }
 
-    double scale = 1;
-    double offset = 0;
-
     if (prevIndex == -1)
         prevIndex = 0;
@@ -123 +120 @@
     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()));

trunk/Source/WebCore/platform/Length.h

@@ -423 +423 @@
 
     if (from.isZero() && isZero())
-        return *this;
+        return progress ? *this : from; // Pick up 'auto' from 'from' if progress is zero.
 
     LengthType resultType = type();