Bug 159959 - CSSStyleDeclaration.setProperty() should be able to unset "important" on a property
Summary: CSSStyleDeclaration.setProperty() should be able to unset "important" on a pr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: WebExposed
Depends on:
Blocks:
 
Reported: 2016-07-19 16:40 PDT by Chris Dumez
Modified: 2016-07-20 11:02 PDT (History)
8 users (show)

See Also:


Attachments
WIP patch (3.27 KB, patch)
2016-07-19 16:53 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP patch (3.28 KB, patch)
2016-07-19 16:53 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (1.10 MB, application/zip)
2016-07-19 17:29 PDT, Build Bot
no flags Details
Patch (6.31 KB, patch)
2016-07-20 10:08 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-07-19 16:40:12 PDT
CSSStyleDeclaration.setProperty() should be able to unset "important" on a property. Currently, if you do the following:
element.style.setProperty("color", "red", "important");
element.style.setProperty("color", "red", "");

It does not unset the "important" flag on the color property.

Relevant specification:
- https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty
- https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute

Firefox and Chrome match the specification here.
Comment 1 Chris Dumez 2016-07-19 16:53:09 PDT
Created attachment 284070 [details]
WIP patch
Comment 2 Chris Dumez 2016-07-19 16:53:59 PDT
Created attachment 284072 [details]
WIP patch
Comment 3 Build Bot 2016-07-19 17:29:05 PDT
Comment on attachment 284072 [details]
WIP patch

Attachment 284072 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1709355

New failing tests:
fast/mediastream/MediaStream-video-element-displays-buffer.html
Comment 4 Build Bot 2016-07-19 17:29:08 PDT
Created attachment 284073 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 5 Chris Dumez 2016-07-20 10:01:21 PDT
Comment on attachment 284072 [details]
WIP patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284072&action=review

> Source/WebCore/css/StyleProperties.cpp:-887
> -    // Only add properties that have no !important counterpart present

Looks like this code was added via https://bugs.webkit.org/show_bug.cgi?id=60007 whose layout test is LayoutTests/fast/css/important-js-override.html. However, the behavior covered in LayoutTests/fast/css/important-js-override.html does not match the latest specification and this test is failing in both Firefox and Chrome.
Comment 6 Chris Dumez 2016-07-20 10:08:25 PDT
Created attachment 284118 [details]
Patch
Comment 7 WebKit Commit Bot 2016-07-20 11:02:23 PDT
Comment on attachment 284118 [details]
Patch

Clearing flags on attachment: 284118

Committed r203460: <http://trac.webkit.org/changeset/203460>
Comment 8 WebKit Commit Bot 2016-07-20 11:02:31 PDT
All reviewed patches have been landed.  Closing bug.