Bug 96658

Summary: REGRESSION: transition doesn’t always override transition-property
Product: WebKit Reporter: Lea Verou <lea>
Component: CSSAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, dino, macpherson, menard, simon.fraser, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://result.dabblet.com/gist/3715069
Attachments:
Description Flags
Patch dino: review+

Description Lea Verou 2012-09-13 08:56:54 PDT
As you can see in the testcase, there is no smooth transition because the transition shorthand does not seem to override the first transition-property, even though it should, and it does in every other engine and older versions of WebKit. Chrome 19 and Safari 6.0 do not have this issue. It can be reproduced in Chrome > 19 and WebKit nightlies. I can also reproduce it in my Safari 6.0, but not BrowserStack’s Safari 6.0, so I'm not sure whether it occurs there.

Steps to reproduce:
- Go to testcase
- Hover over the gold paragraph
- Observe how there is no smooth transition

Things that fix the issue:
- Explicitly adding the transition-property value in the shorthand. i.e. converting transition: 2s; to transition: 2s all; or transition 2s background;
- Merging the two rules

Things that don't fix the issue:
- Increasing the specificity of the second rule
- Adding a separate transition-property declaration in the second rule WITHOUT removing the first one

Other observations:
- Other transition-* properties like transition-duration or transition-delay don't seem to exhibit this issue.
Comment 1 Simon Fraser (smfr) 2012-09-13 11:34:07 PDT
That does seem like a bug.
Comment 2 Simon Fraser (smfr) 2012-09-13 17:25:43 PDT
Interesting. In debug, we hit:
ASSERTION FAILED: prop >= firstCSSProperty && prop < (firstCSSProperty + numCSSProperties)
Comment 3 Simon Fraser (smfr) 2012-09-13 18:42:57 PDT
When parsing the shorthand, CSSParser::parseTransitionShorthand() fills in unsupplied values with the "initial" value:

    // Fill in any remaining properties with the initial value.
    for (i = 0; i < numProperties; ++i) {
        if (!parsedProperty[i])
            addAnimationValue(values[i], cssValuePool().createImplicitInitialValue());
    }

and later on we take this to imply that we should use Animation::initialAnimationProperty() which is CSSPropertyInvalid. Maybe that should be All?
Comment 4 Simon Fraser (smfr) 2012-09-14 10:43:21 PDT
Regressed with http://trac.webkit.org/changeset/113225 when Alexis changed initialAnimationProperty() to return CSSPropertyInvalid rather than cAnimateAll.

Also, the use of CSSPropertyID in platform/animation code is a layering violation.
Comment 5 Simon Fraser (smfr) 2012-09-14 14:11:17 PDT
Possibly seeing this (the assertion at least) on
http://davidcelis.com/blog/2012/09/06/stop-validating-email-addresses-with-regex/?utm_source=rubyweekly&utm_medium=email
Comment 6 Simon Fraser (smfr) 2012-09-14 14:41:09 PDT
Created attachment 164228 [details]
Patch
Comment 7 Simon Fraser (smfr) 2012-09-14 14:41:33 PDT
<rdar://problem/12302561>
Comment 8 Simon Fraser (smfr) 2012-09-14 14:53:00 PDT
http://trac.webkit.org/changeset/128656
Comment 9 Simon Fraser (smfr) 2012-09-14 15:26:19 PDT
Build fix in http://trac.webkit.org/changeset/128660