REGRESSION (Safari 10.1): When 'transition' contains -ms-transform, transform-origin...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 May 2017 20:16:01 +0000 (20:16 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 May 2017 20:16:01 +0000 (20:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171250
<rdar://problem/31827243>

Reviewed by Geoffrey Garen.

Source/WebCore:

We were mapping unknown properties to 'all' animation. With this patch we ignore them instead.
The patch also implements roundtripping of unknown properties via CSSOM, matching Blink and Gecko.

Test: transitions/transition-unknown-property-ignore.html

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::createTransitionPropertyValue):

    Return the correct name for unknown properties.

* css/CSSToStyleMap.cpp:
(WebCore::CSSToStyleMap::mapAnimationProperty):

    Map any unknown property to AnimateUnknownProperty mode instead of falling back to the default of AnimateAll.
    Save the unknown property name so we can roundtrip it properly.

* page/animation/CompositeAnimation.cpp:
(WebCore::CompositeAnimation::updateTransitions):

    Ignore AnimateUnknownProperty like AnimateNone.

* platform/animation/Animation.h:
(WebCore::Animation::unknownProperty):
(WebCore::Animation::setUnknownProperty):

LayoutTests:

* transitions/transition-unknown-property-ignore-expected.txt: Added.
* transitions/transition-unknown-property-ignore.html: Added.
* transitions/transitions-parsing-expected.txt:
* transitions/transitions-parsing.html:

    Update the roundtrip expectations for unknown properties. The new results match Blink and Gecko.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@216204 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/transitions/transition-unknown-property-ignore-expected.txt [new file with mode: 0644]
LayoutTests/transitions/transition-unknown-property-ignore.html [new file with mode: 0644]
LayoutTests/transitions/transitions-parsing-expected.txt
LayoutTests/transitions/transitions-parsing.html
Source/WebCore/ChangeLog
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/CSSToStyleMap.cpp
Source/WebCore/page/animation/CompositeAnimation.cpp
Source/WebCore/platform/animation/Animation.h

index 90ffc6b..be74bb7 100644 (file)
@@ -1,3 +1,18 @@
+2017-05-04  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (Safari 10.1): When 'transition' contains -ms-transform, transform-origin is also transitioned
+        https://bugs.webkit.org/show_bug.cgi?id=171250
+        <rdar://problem/31827243>
+
+        Reviewed by Geoffrey Garen.
+
+        * transitions/transition-unknown-property-ignore-expected.txt: Added.
+        * transitions/transition-unknown-property-ignore.html: Added.
+        * transitions/transitions-parsing-expected.txt:
+        * transitions/transitions-parsing.html:
+
+            Update the roundtrip expectations for unknown properties. The new results match Blink and Gecko.
+
 2017-05-04  Chris Dumez  <cdumez@apple.com>
 
         Reformat / clean up Event.idl
diff --git a/LayoutTests/transitions/transition-unknown-property-ignore-expected.txt b/LayoutTests/transitions/transition-unknown-property-ignore-expected.txt
new file mode 100644 (file)
index 0000000..7fcc937
--- /dev/null
@@ -0,0 +1,5 @@
+
+
+Transitioning properties:
+transform
+
diff --git a/LayoutTests/transitions/transition-unknown-property-ignore.html b/LayoutTests/transitions/transition-unknown-property-ignore.html
new file mode 100644 (file)
index 0000000..bd5bd12
--- /dev/null
@@ -0,0 +1,44 @@
+<style type="text/css">
+.container {
+    transition: -ms-transform 0.1s ease, transform 0.1s ease;
+    transform-origin: 100% 100%;
+}
+
+.container-transform {
+    transform: scale3d(1.3, 1.3, 1);
+    transform-origin: 0% 0%;
+}
+
+.box {
+    width: 100px;
+    height: 100px;
+    background-color: red;
+}
+</style>
+
+<script type='text/javascript'>
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function test() {
+    document.querySelector(".container").ontransitionend = (ev) => {
+        log.innerHTML += ev.propertyName +"<br>";
+        if (window.testRunner)
+            setTimeout(() => testRunner.notifyDone(), 0);
+    }
+    a.parentNode.classList.add('container-transform');
+}
+
+</script>
+
+<body onload="test()">
+<div class="container">
+<div id="a" class="box">
+</div>
+</div>
+<br><br>
+Transitioning properties:
+<div id=log></div>
index 57e5371..24b9897 100644 (file)
@@ -46,17 +46,17 @@ PASS style.webkitTransitionProperty is 'font-size, all, color'
 PASS computedStyle.webkitTransitionProperty is 'font-size, all, color'
 Invalid transition-property values.
 PASS style.transitionProperty is 'solid, font-size'
-PASS computedStyle.transitionProperty is 'all'
+PASS computedStyle.transitionProperty is 'solid, font-size'
 PASS style.webkitTransitionProperty is 'solid, font-size'
-PASS computedStyle.webkitTransitionProperty is 'all'
+PASS computedStyle.webkitTransitionProperty is 'solid, font-size'
 PASS style.transitionProperty is 'solid, left'
-PASS computedStyle.transitionProperty is 'all'
+PASS computedStyle.transitionProperty is 'solid, left'
 PASS style.webkitTransitionProperty is 'solid, left'
-PASS computedStyle.webkitTransitionProperty is 'all'
+PASS computedStyle.webkitTransitionProperty is 'solid, left'
 PASS style.transitionProperty is 'solid'
-PASS computedStyle.transitionProperty is 'all'
+PASS computedStyle.transitionProperty is 'solid'
 PASS style.webkitTransitionProperty is 'solid'
-PASS computedStyle.webkitTransitionProperty is 'all'
+PASS computedStyle.webkitTransitionProperty is 'solid'
 PASS style.transitionProperty is ''
 PASS computedStyle.transitionProperty is 'all'
 PASS style.webkitTransitionProperty is ''
@@ -423,9 +423,9 @@ PASS computedStyle.transition is 'all 0s ease 0s'
 PASS style.webkitTransition is ''
 PASS computedStyle.webkitTransition is 'all 0s ease 0s'
 PASS style.transition is 'widthFoo'
-PASS computedStyle.transition is 'all 0s ease 0s'
+PASS computedStyle.transition is 'widthFoo 0s ease 0s'
 PASS style.webkitTransition is 'widthFoo'
-PASS computedStyle.webkitTransition is 'all 0s ease 0s'
+PASS computedStyle.webkitTransition is 'widthFoo 0s ease 0s'
 PASS style.transition is ''
 PASS computedStyle.transition is 'all 0s ease 0s'
 PASS style.webkitTransition is ''
index 82fa649..6e0d532 100644 (file)
@@ -90,21 +90,21 @@ style.transitionProperty = "";
 
 style.transitionProperty = "solid, font-size";
 shouldBe("style.transitionProperty", "'solid, font-size'");
-shouldBe("computedStyle.transitionProperty", "'all'");
+shouldBe("computedStyle.transitionProperty", "'solid, font-size'");
 shouldBe("style.webkitTransitionProperty", "'solid, font-size'");
-shouldBe("computedStyle.webkitTransitionProperty", "'all'");
+shouldBe("computedStyle.webkitTransitionProperty", "'solid, font-size'");
 
 style.transitionProperty = "solid, left";
 shouldBe("style.transitionProperty", "'solid, left'");
-shouldBe("computedStyle.transitionProperty", "'all'");
+shouldBe("computedStyle.transitionProperty", "'solid, left'");
 shouldBe("style.webkitTransitionProperty", "'solid, left'");
-shouldBe("computedStyle.webkitTransitionProperty", "'all'");
+shouldBe("computedStyle.webkitTransitionProperty", "'solid, left'");
 
 style.transitionProperty = "solid";
 shouldBe("style.transitionProperty", "'solid'");
-shouldBe("computedStyle.transitionProperty", "'all'");
+shouldBe("computedStyle.transitionProperty", "'solid'");
 shouldBe("style.webkitTransitionProperty", "'solid'");
-shouldBe("computedStyle.webkitTransitionProperty", "'all'");
+shouldBe("computedStyle.webkitTransitionProperty", "'solid'");
 
 style.transitionProperty = '';
 
@@ -668,9 +668,9 @@ shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
 
 style.transition = "widthFoo";
 shouldBe("style.transition", "'widthFoo'");
-shouldBe("computedStyle.transition", "'all 0s ease 0s'");
+shouldBe("computedStyle.transition", "'widthFoo 0s ease 0s'");
 shouldBe("style.webkitTransition", "'widthFoo'");
-shouldBe("computedStyle.webkitTransition", "'all 0s ease 0s'");
+shouldBe("computedStyle.webkitTransition", "'widthFoo 0s ease 0s'");
 
 style.transition = '';
 
index 7eba93a..2b55c1f 100644 (file)
@@ -1,3 +1,36 @@
+2017-05-04  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (Safari 10.1): When 'transition' contains -ms-transform, transform-origin is also transitioned
+        https://bugs.webkit.org/show_bug.cgi?id=171250
+        <rdar://problem/31827243>
+
+        Reviewed by Geoffrey Garen.
+
+        We were mapping unknown properties to 'all' animation. With this patch we ignore them instead.
+        The patch also implements roundtripping of unknown properties via CSSOM, matching Blink and Gecko.
+
+        Test: transitions/transition-unknown-property-ignore.html
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::createTransitionPropertyValue):
+
+            Return the correct name for unknown properties.
+
+        * css/CSSToStyleMap.cpp:
+        (WebCore::CSSToStyleMap::mapAnimationProperty):
+
+            Map any unknown property to AnimateUnknownProperty mode instead of falling back to the default of AnimateAll.
+            Save the unknown property name so we can roundtrip it properly.
+
+        * page/animation/CompositeAnimation.cpp:
+        (WebCore::CompositeAnimation::updateTransitions):
+
+            Ignore AnimateUnknownProperty like AnimateNone.
+
+        * platform/animation/Animation.h:
+        (WebCore::Animation::unknownProperty):
+        (WebCore::Animation::setUnknownProperty):
+
 2017-05-04  Chris Dumez  <cdumez@apple.com>
 
         Clean up MutationRecord.idl
index 419d498..c05cc3c 100644 (file)
@@ -1199,11 +1199,18 @@ static Ref<CSSValue> valueForGridPosition(const GridPosition& position)
 
 static Ref<CSSValue> createTransitionPropertyValue(const Animation& animation)
 {
-    if (animation.animationMode() == Animation::AnimateNone)
+    switch (animation.animationMode()) {
+    case Animation::AnimateNone:
         return CSSValuePool::singleton().createIdentifierValue(CSSValueNone);
-    if (animation.animationMode() == Animation::AnimateAll)
+    case Animation::AnimateAll:
         return CSSValuePool::singleton().createIdentifierValue(CSSValueAll);
-    return CSSValuePool::singleton().createValue(getPropertyNameString(animation.property()), CSSPrimitiveValue::CSS_STRING);
+    case Animation::AnimateSingleProperty:
+        return CSSValuePool::singleton().createValue(getPropertyNameString(animation.property()), CSSPrimitiveValue::CSS_STRING);
+    case Animation::AnimateUnknownProperty:
+        return CSSValuePool::singleton().createValue(animation.unknownProperty(), CSSPrimitiveValue::CSS_STRING);
+    }
+    ASSERT_NOT_REACHED();
+    return CSSValuePool::singleton().createIdentifierValue(CSSValueNone);
 }
 
 static Ref<CSSValueList> transitionPropertyValue(const AnimationList* animationList)
index 85c44a2..e68b6db 100644 (file)
@@ -443,13 +443,21 @@ void CSSToStyleMap::mapAnimationProperty(Animation& animation, const CSSValue& v
     if (primitiveValue.valueID() == CSSValueAll) {
         animation.setAnimationMode(Animation::AnimateAll);
         animation.setProperty(CSSPropertyInvalid);
-    } else if (primitiveValue.valueID() == CSSValueNone) {
+        return;
+    }
+    if (primitiveValue.valueID() == CSSValueNone) {
         animation.setAnimationMode(Animation::AnimateNone);
         animation.setProperty(CSSPropertyInvalid);
-    } else if (primitiveValue.propertyID() != CSSPropertyInvalid) {
-        animation.setAnimationMode(Animation::AnimateSingleProperty);
-        animation.setProperty(primitiveValue.propertyID());
+        return;
+    }
+    if (primitiveValue.propertyID() == CSSPropertyInvalid) {
+        animation.setAnimationMode(Animation::AnimateUnknownProperty);
+        animation.setProperty(CSSPropertyInvalid);
+        animation.setUnknownProperty(primitiveValue.stringValue());
+        return;
     }
+    animation.setAnimationMode(Animation::AnimateSingleProperty);
+    animation.setProperty(primitiveValue.propertyID());
 }
 
 void CSSToStyleMap::mapAnimationTimingFunction(Animation& animation, const CSSValue& value)
index daddda4..77d1820 100644 (file)
@@ -97,7 +97,7 @@ void CompositeAnimation::updateTransitions(RenderElement* renderer, const Render
             bool isActiveTransition = !m_suspended && (animation.duration() || animation.delay() > 0);
 
             Animation::AnimationMode mode = animation.animationMode();
-            if (mode == Animation::AnimateNone)
+            if (mode == Animation::AnimateNone || mode == Animation::AnimateUnknownProperty)
                 continue;
 
             CSSPropertyID prop = animation.property();
index 8f5ad93..669cc24 100644 (file)
@@ -109,7 +109,7 @@ public:
 
     double delay() const { return m_delay; }
 
-    enum AnimationMode { AnimateAll, AnimateNone, AnimateSingleProperty };
+    enum AnimationMode { AnimateAll, AnimateNone, AnimateSingleProperty, AnimateUnknownProperty };
 
     enum AnimationDirection {
         AnimationDirectionNormal,
@@ -130,6 +130,7 @@ public:
     Style::ScopeOrdinal nameStyleScopeOrdinal() const { return m_nameStyleScopeOrdinal; }
     EAnimPlayState playState() const { return static_cast<EAnimPlayState>(m_playState); }
     CSSPropertyID property() const { return m_property; }
+    const String& unknownProperty() const { return m_unknownProperty; }
     TimingFunction* timingFunction() const { return m_timingFunction.get(); }
     AnimationMode animationMode() const { return m_mode; }
 #if ENABLE(CSS_ANIMATIONS_LEVEL_2)
@@ -149,6 +150,7 @@ public:
     }
     void setPlayState(EAnimPlayState d) { m_playState = d; m_playStateSet = true; }
     void setProperty(CSSPropertyID t) { m_property = t; m_propertySet = true; }
+    void setUnknownProperty(const String& property) { m_unknownProperty = property; }
     void setTimingFunction(RefPtr<TimingFunction>&& function) { m_timingFunction = WTFMove(function); m_timingFunctionSet = true; }
     void setAnimationMode(AnimationMode mode) { m_mode = mode; }
 #if ENABLE(CSS_ANIMATIONS_LEVEL_2)
@@ -176,6 +178,7 @@ private:
     String m_name;
     Style::ScopeOrdinal m_nameStyleScopeOrdinal { Style::ScopeOrdinal::Element };
     CSSPropertyID m_property;
+    String m_unknownProperty;
     AnimationMode m_mode;
     double m_iterationCount;
     double m_delay;