RESOLVED FIXED94923
Web Inspector: Incorrect property override computation when !important is involved 
https://bugs.webkit.org/show_bug.cgi?id=94923
Summary Web Inspector: Incorrect property override computation when !important is inv...
Alexander Pavlov (apavlov)
Reported 2012-08-24 04:50:26 PDT
What steps will reproduce the problem? 1. Have an element with an !important css rule and a conflicting inline style 2. Inspect element What is the expected result? I'm expecting the element style to be crossed out as it is not the active style What happens instead? The element style is not crossed out. This can make it very hard to figure out what is going on, as the !important rule can be much farther down in the list of styles. Test case: <html> <head><style type="text/css"> p.foo { display: block !important } </style><body> <p class="foo bar" style="display:none">Hello, world!</p> </body></html> Upstreaming http://code.google.com/p/chromium/issues/detail?id=144209
Attachments
Patch (9.01 KB, patch)
2012-08-24 07:35 PDT, Alexander Pavlov (apavlov) 		no flags
DetailsFormatted DiffDiff
Patch (10.38 KB, patch)
2012-08-24 08:57 PDT, Alexander Pavlov (apavlov) 		no flags
DetailsFormatted DiffDiff
Patch (9.58 KB, patch)
2012-08-27 01:50 PDT, Alexander Pavlov (apavlov) 		vsevik: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexander Pavlov (apavlov)
Comment 1 2012-08-24 07:35:18 PDT
Created attachment 160421 [details] Patch
Alexander Pavlov (apavlov)
Comment 2 2012-08-24 08:57:07 PDT
Created attachment 160429 [details] Patch
Vsevolod Vlasov
Comment 3 2012-08-24 09:59:08 PDT
Comment on attachment 160429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160429&action=review > Source/WebCore/inspector/front-end/StylesSidebarPane.js:521 > + styleRule.importantProperties = {}; This could be a local variable, see below. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:535 > + if (!isImportant && usedProperties.hasOwnProperty(canonicalName)) I would add || foundImportantProperties.hasOwnProperty(canonicalName) to the condition so that we don't need to store styleRule.importantProperties for the next loop iterations. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:549 > + for (var j = 0; j < i; ++j) { As discussed we can avoid inner loop by using a map to remember the only rule from which this particular property is used.
Vsevolod Vlasov
Comment 4 2012-08-24 09:59:57 PDT
Please update bug title with a more generic description of this bug.
Alexander Pavlov (apavlov)
Comment 5 2012-08-24 10:11:20 PDT
Comment on attachment 160429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160429&action=review >> Source/WebCore/inspector/front-end/StylesSidebarPane.js:535 >> + if (!isImportant && usedProperties.hasOwnProperty(canonicalName)) > > I would add || foundImportantProperties.hasOwnProperty(canonicalName) to the condition so that we don't need to store styleRule.importantProperties for the next loop iterations. This check is done a lot earlier, on the line 529 above. We still need to store importantProperties unless we get a map in, as discussed. >> Source/WebCore/inspector/front-end/StylesSidebarPane.js:549 >> + for (var j = 0; j < i; ++j) { > > As discussed we can avoid inner loop by using a map to remember the only rule from which this particular property is used. Yes, will fix.
Alexander Pavlov (apavlov)
Comment 6 2012-08-27 01:50:46 PDT
Created attachment 160670 [details] Patch
Vsevolod Vlasov
Comment 7 2012-08-27 02:32:49 PDT
Comment on attachment 160670 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160670&action=review > Source/WebCore/inspector/front-end/StylesSidebarPane.js:528 > + // No need to check any further if we know an !important value has already been seen. I don't think this comment is needed. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:534 > + // Property is not important in this rule and has encountered earlier - do nothing. I don't think this comment is needed.
Alexander Pavlov (apavlov)
Comment 8 2012-08-27 02:35:51 PDT
Committed r126737: <http://trac.webkit.org/changeset/126737>
Note You need to log in before you can comment on or make changes to this bug.