Bug 104146 - Web Inspector: Add shortcut to set visibility:hidden on elements in the ElementsPanel
Summary: Web Inspector: Add shortcut to set visibility:hidden on elements in the Eleme...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-05 11:30 PST by egraether
Modified: 2012-12-07 12:53 PST (History)
13 users (show)

See Also:

Attachments
Patch (6.96 KB, patch)
2012-12-05 11:32 PST, egraether 		no flags Details | Formatted Diff | Diff
Patch (7.40 KB, patch)
2012-12-05 15:08 PST, egraether 		no flags Details | Formatted Diff | Diff
Patch (6.21 KB, patch)
2012-12-07 11:25 PST, egraether 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.