Bug 73941

Summary: Handling of !important in inline style sets is broken
Product: WebKit Reporter: Boris Zbarsky <bzbarsky>
Component: CSSAssignee: Alexey Proskuryakov <ap>
Severity: Normal CC: ap, macpherson, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
proposed fix hyatt: review+

Description Boris Zbarsky 2011-12-06 11:49:31 PST
Consider the attached testcase, which does:

      document.getElementById("x").style.color = "red !important";

Per http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface the behavior should be:

  Similarly for the table below, setting the IDL attribute in the first column must invoke
  setProperty() with as first argument the CSS property given in the second column on the same row,
  as second argument the given value, and no third argument.

and the definition of setProperty says:

  If parsing the value returns null terminate this algorithm.
     Note: value can not include "!important".

WebKit gets the setProperty part right, but not the idl attribute set part.  Gecko and IE9 get the idl attribute part right.
Comment 1 Boris Zbarsky 2011-12-06 11:50:10 PST
Created attachment 118083 [details]
Comment 2 Boris Zbarsky 2011-12-06 11:53:30 PST
Not only that, but if I put a space after the '!' suddenly the color ends up green.  Which is just completely weird, since in CSS syntax a space is allowed after '!', so it's not like this is an accidental behavior due to using the normal CSS parser...
Comment 3 Alexey Proskuryakov 2011-12-07 10:44:38 PST
This behavior is a result of a fix in bug 8223. The claim there was that both IE and Firefox supported "!important" when setting property values via IDL attributes. What that always untrue, or did the behavior change since 2006?

> Not only that, but if I put a space after the '!' suddenly the color ends up green.

Yes, the fix for bug 8223 failed to take this into account.
Comment 4 Alexey Proskuryakov 2011-12-07 11:43:14 PST
Created attachment 118242 [details]
proposed fix

I'm still puzzled why other browsers apparently turned around on this, but even in the absence of an explanation, if seems like we should do the same.
Comment 5 Dave Hyatt 2011-12-07 11:46:19 PST
Comment on attachment 118242 [details]
proposed fix

Comment 6 Alexey Proskuryakov 2011-12-07 11:54:18 PST
Committed <http://trac.webkit.org/changeset/102262>.
Comment 7 Boris Zbarsky 2011-12-07 11:59:16 PST
Ah, interesting.

IE9 is weird.  On the testcase in this bug (even if I add a <body> to make their old parser happy), it ignores the style set in all modes.  On the testcase in bug 8223, it sets the style in quirks, IE7, and IE8 modes, but not in IE9 standards mode.  Do you see that too?

As for Gecko, it accepted all sorts of gunk in inline style until Gecko 1.9 as long as it started with a valid value followed by '!' or ';' (for example, style.color = "red; flowery petals" would have parsed, as would have "red! for it is so pretty").  This was fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=383075

Thanks for the quick fix!
Comment 8 Alexey Proskuryakov 2011-12-07 13:36:15 PST
Thanks for the info. I didn't test IE very closely, just checked that your test passed.

We'll see if this change breaks any content.
Comment 9 Csaba Osztrogon√°c 2011-12-07 23:34:26 PST
And an unreviewed buildfix landed in http://trac.webkit.org/changeset/102318 ... :(