Bug 110011

<rdar://problem/13232242>

I can confirm it on both Chrome Canary 27.0.1415.0 and WebKit nightly r143127.

Created attachment 190788 [details] Patch

Comment on attachment 190788 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190788&action=review Minor typos and grammar in changelog. I'm happy with the approach. As you said on IRC, antti or kling should probably double check. > Source/WebCore/ChangeLog:11 > + identically parse the two versions which means that when we populate StylePropertySet Suggestion: break sentence after "two versions". e.g "..parse the two versions. This means that..." > Source/WebCore/ChangeLog:15 > + prefixed), each properties have their own id and we know handle them in Suggestion: again, break after "prefixed)". And then "Each property has its own id and we NOW handle them..." (typo for know/now) > Source/WebCore/ChangeLog:16 > + the parsing code as disctints properties and add both versions to the typo: distinct (singular) > Source/WebCore/ChangeLog:19 > + prefixed and the unprefixed entries. Last point the style resolution Typo: "Last point" -> "Finally," > Source/WebCore/ChangeLog:22 > + and the unprefixed versions are not resolved. This is to avoid creating > + two times animation objects for the resolved style. "This is to avoid creating the animation objects two times for the resolved style" > Source/WebCore/ChangeLog:34 > + (WebCore::CSSParser::parseTransitionShorthand): add both prefixed and Nit, "add" -> "Add" > Source/WebCore/ChangeLog:52 > + (WebCore::StylePropertySet::removePrefixedOrUnprefixedProperty): Remove > + also the prefixed or unprefixed shorthand if it exists. Nit: "Also remove..." > Source/WebCore/ChangeLog:58 > + (WebCore::StylePropertySet::setPrefixedOrUnPrefixedProperty): If it > + exists a unprefixed or prefixed counterpart of the property we're > + trying to set then we update the other one. "If a unprefixed or prefixed counterpart of the the property we're trying to set exists then we update the other one" > Source/WebCore/ChangeLog:74 > + unprefixed property we do not want to resolve two times and create "do not want to resolve twice and create..." > Source/WebCore/css/CSSParser.cpp:1539 > +void CSSParser::addPrefixedAndUnPrefixedProperty(CSSPropertyID propId, PassRefPtr<CSSValue> value, bool important, bool implicit) This name is a little cumbersome, but I'm not coming up with great alternatives. addPropertyWithPrefixingVariant > Source/WebCore/css/CSSProperty.h:95 > +inline CSSPropertyID prefixedOrUnPrefixedPropertyId(CSSPropertyID propId) Similarly, how about prefixingVariantForPropertyId > Source/WebCore/css/StylePropertySet.cpp:588 > + bool ret = removePropertiesInSet(shorthand.properties(), shorthand.length()); > + > + CSSPropertyID prefixedOrUnprefixedID = prefixedOrUnPrefixedPropertyId(propertyID); > + if (prefixedOrUnprefixedID == propertyID) > + return ret; We should have a better name for the "ret" variable. > Source/WebCore/css/StylePropertySet.cpp:590 > + StylePropertyShorthand prefixedOrUnprefixedShorthand = shorthandForProperty(prefixedOrUnprefixedID); And here is another case where it would be nice to avoid the ugly "prefixedOrUnprefixed" (it also avoids the issue of where to capitalize :) shorthandPrefixingVariant > Source/WebCore/css/StylePropertySet.cpp:709 > +void StylePropertySet::appendPrefixedAndUnprefixed(const CSSProperty& property) appendPrefixingVariant

Comment on attachment 190788 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190788&action=review >> Source/WebCore/ChangeLog:22 >> + two times animation objects for the resolved style. > > "This is to avoid creating the animation objects two times for the resolved style" errr... "creating the animation objects twice for" is better >> Source/WebCore/ChangeLog:58 >> + trying to set then we update the other one. > > "If a unprefixed or prefixed counterpart of the the property we're trying to set exists then we update the other one" "set exists, then" (add comma) > Source/WebCore/ChangeLog:75 > + animations in double, therefore when we try to resolve the unprefixed "and create duplicate animations. Therefore.."

Created attachment 190994 [details] Patch

Comment on attachment 190994 [details] Patch r=me

Comment on attachment 190994 [details] Patch Clearing flags on attachment: 190994 Committed r144626: <http://trac.webkit.org/changeset/144626>

All reviewed patches have been landed. Closing bug.

I confirmed that this bug is fixed on Chromium 27.0.1431.0 (186272). Thanks!

I want to revert this patch. It causes element.style.cssText to report both prefixed and unprefixed versions of animation and transition properties, which is wrong. Also, why was this all specific to just animations and transitions, and not other prefixed properties?

This is mostly reverted by the fix for https://bugs.webkit.org/show_bug.cgi?id=157569.