Bug 151066 – Web Inspector: Global Breakpoints should always be visible

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

* SUMMARY
Global Breakpoints should always be visible. There are only two global breakpoints, so there isn't much utility in making them filterable.

The current behavior is also inconsistent and confusing. Some examples:

1) Filter text "Global" results in an empty global breakpoints folder in trunk. In Safari 9.0 the child tree elements are also shown.
2) Filter text "All Exceptions" results in nothing.
3) Enabling "Show resources with issues" hides Global Breakpoints.
4) Enabling "Show resources with breakpoints" shows Global Breakpoints.

[Radar WebKit Bug Importer]

<rdar://problem/23474149>

[Matt Baker]

Created attachment 266862 [details]
[Patch] Proposed Fix

[Timothy Hatcher]

Comment on attachment 266862 [details]
[Patch] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=266862&action=review

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:180
> +    suppressFiltering(treeElements)

This does not sound like a function that acts on tree elements. suppressFilteringOnTreeElements?

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:183
> +        if (!Array.isArray(treeElements))
> +            treeElements = [treeElements];

I think Joe has wanted to get away from this pattern. Just assert.

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:514
> +            if (!currentTreeElement.hidden && !currentTreeElement[WebInspector.NavigationSidebarPanel.SuppressFiltering]) {

It seems odd to check this here, since this code hides the empty placeholder, which would never be used if there is even a single tree element with the SuppressFiltering symbol. I think the hidden check is sufficient here.

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:733
> +WebInspector.NavigationSidebarPanel.SuppressFiltering = Symbol("supresss-filtering");

SuppressFilteringSymbol?

[Matt Baker]

(In reply to comment #3)
> Comment on attachment 266862 [details]
> [Patch] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=266862&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:180
> > +    suppressFiltering(treeElements)
> 
> This does not sound like a function that acts on tree elements.
> suppressFilteringOnTreeElements?
> 
> > Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:183
> > +        if (!Array.isArray(treeElements))
> > +            treeElements = [treeElements];
> 
> I think Joe has wanted to get away from this pattern. Just assert.

+1! I think we should assert more, and massage inputs less.

> > Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:514
> > +            if (!currentTreeElement.hidden && !currentTreeElement[WebInspector.NavigationSidebarPanel.SuppressFiltering]) {
> 
> It seems odd to check this here, since this code hides the empty
> placeholder, which would never be used if there is even a single tree
> element with the SuppressFiltering symbol. I think the hidden check is
> sufficient here.

This ensures a visible element with the SuppressFilteringSymbol doesn't prevent the placeholder from being shown. In the case of DebuggerSidebarPanel, if all breakpoint elements are hidden by filters, the "No Filter Results" message should be displayed, even though global breakpoints are still visible.

> > Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:733
> > +WebInspector.NavigationSidebarPanel.SuppressFiltering = Symbol("supresss-filtering");
> 
> SuppressFilteringSymbol?

[Matt Baker]

Created attachment 266910 [details]
[Image] Elements with SuppressFilteringSymbol

Placeholder message appears when all filterable elements (those with SuppressFilteringSymbol equal to undefined or false) are hidden

[Matt Baker]

Created attachment 266911 [details]
[Patch] Proposed Fix

[Timothy Hatcher]

(In reply to comment #5)
> Created attachment 266910 [details]
> [Image] Elements with SuppressFilteringSymbol
> 
> Placeholder message appears when all filterable elements (those with
> SuppressFilteringSymbol equal to undefined or false) are hidden

Okay, makes sense. This will change once sections are added.

[WebKit Commit Bot]

Comment on attachment 266911 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 266911

Committed r193771: <http://trac.webkit.org/changeset/193771>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.