[webkit-reviews] review denied: [Bug 104900] Web Inspector: Link to W3C specification of CSS properties : [Attachment 179259] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 104900: Web Inspector: Link to W3C specification of CSS properties
https://bugs.webkit.org/show_bug.cgi?id=104900

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=179259&action=review


> Source/WebCore/inspector/front-end/StylesSidebarPane.js:33
> + * @implements {WebInspector.ContextMenu.Provider}

Only use ContextMenu.Provider on generic targets. UISourceCode, etc. In other
cases simply create and populate it by hand.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:165
> +	   var url =
WebInspector.StylesSidebarPane.W3OrgData.getHelpURL(propertyName);

No get prefixes in WebKit plz.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2617
> +WebInspector.StylesSidebarPane.W3OrgData = {}

Does not look like a good idea to increase file size from 2600 lines to 2900.
Extract?

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2623
> +WebInspector.StylesSidebarPane.W3OrgData.getHelpURL = function(propertyName)


Here and below: no get prefixes please.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2706
> +    "color": { m: "color", a: "foreground" },

Please don't create new metainfo on the properties - we already have
WebInspector.CSSKeywordCompletions that you could annotate with spec info.