Bug 104146

Summary: Web Inspector: Add shortcut to set visibility:hidden on elements in the ElementsPanel
Product: WebKit Reporter: egraether
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, caseq, dglazkov, egraether, keishi, loislo, nduca, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description egraether 2012-12-05 11:30:47 PST
Web Inspector: Add shortcut to set visibility:hidden on elements in the ElementsPanel
Comment 1 egraether 2012-12-05 11:32:56 PST
Created attachment 177802 [details]
Patch
Comment 2 egraether 2012-12-05 15:08:36 PST
Created attachment 177840 [details]
Patch
Comment 3 egraether 2012-12-05 15:12:38 PST
pfeldman, thanks for the feedback.

I rewrote the patch to using cssStyleModel.
Comment 4 WebKit Review Bot 2012-12-05 15:54:55 PST
Comment on attachment 177840 [details]
Patch

Attachment 177840 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15147671

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Comment 5 Pavel Feldman 2012-12-07 05:10:53 PST
Comment on attachment 177840 [details]
Patch

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.
Comment 6 egraether 2012-12-07 11:25:24 PST
Created attachment 178244 [details]
Patch
Comment 7 egraether 2012-12-07 11:28:55 PST
(In reply to comment #5)

I updated the patch:

> (From update of attachment 177840 [details])
> 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.

done

> > 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.

I found a spot in ElementsTreeOutline.js that seemed to fit.
Comment 8 WebKit Review Bot 2012-12-07 12:53:33 PST
Comment on attachment 178244 [details]
Patch

Clearing flags on attachment: 178244

Committed r136974: <http://trac.webkit.org/changeset/136974>
Comment 9 WebKit Review Bot 2012-12-07 12:53:37 PST
All reviewed patches have been landed.  Closing bug.