[webkit-reviews] review denied: [Bug 94923] Web Inspector: Incorrect !important handling with inline styles : [Attachment 160429] Patch

[bugzilla-daemon at webkit.org]

Vsevolod Vlasov <vsevik at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 94923: Web Inspector: Incorrect !important handling with inline styles
https://bugs.webkit.org/show_bug.cgi?id=94923

Attachment 160429: Patch
https://bugs.webkit.org/attachment.cgi?id=160429&action=review

------- Additional Comments from Vsevolod Vlasov <vsevik at chromium.org>
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.