[webkit-reviews] review denied: [Bug 104146] Web Inspector: Add shortcut to set visibility:hidden on elements in the ElementsPanel : [Attachment 177840] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 7 05:08:25 PST 2012


Pavel Feldman <pfeldman at chromium.org> has denied egraether at chromium.org's
request for review:
Bug 104146: Web Inspector: Add shortcut to set visibility:hidden on elements in
the ElementsPanel
https://bugs.webkit.org/show_bug.cgi?id=104146

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

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


> Source/WebCore/inspector/front-end/DOMAgent.js:777
> +	   WebInspector.cssModel.getInlineStylesAsync(this.id,
updateInlineVisibility);

Lets keep CSS -> DOM dependency one way. I.e. toggleInlineVisibility should be
in CSSStyleModel and should accept node id (or node) as a parameter.

> Source/WebCore/inspector/front-end/treeoutline.js:377
> +    else if (event.keyCode === WebInspector.KeyboardShortcut.Keys.H.code)

Hide is not generic enough to be dispatched from treeoutline. You should add
explicit listener in the ElementsPanel.js that does it.


More information about the webkit-reviews mailing list