* 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.
<rdar://problem/23474149>
Created attachment 266862 [details] [Patch] Proposed Fix
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?
(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?
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
Created attachment 266911 [details] [Patch] Proposed Fix
(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.
Comment on attachment 266911 [details] [Patch] Proposed Fix Clearing flags on attachment: 266911 Committed r193771: <http://trac.webkit.org/changeset/193771>
All reviewed patches have been landed. Closing bug.