Bug 210672 – Web Inspector: removing a `WI.TreeElement` in a `WI.NavigationSidebar` doesn't check if the `WI.TreeOutline` still matches the current filter

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

[Devin Rousso]

# STEPS TO REPRODUCE
1. inspect any page with a JavaScript resource
2. go to the Sources Tab
3. filter for that JavaScript resource
 => the Breakpoints section should say "No Filter Results"
4. set a breakpoint in that JavaScript resource
 => the Breakpoints section should update to show the new breakpoint
5. delete the breakpoint added in step 4

## EXPECTED

The Breakpoints section should show "No Filter Results".

## ACTUAL

The Breakpoints section is empty.

[Devin Rousso]

Created attachment 396800 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 396800 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:634
> +    _handleTreeElementRemoved(event)
> +    {
> +        this._checkForEmptyFilterResults();

I wonder how much time is spent in `_checkForEmptyFilterResults` during bulk modifications (adds or removes). That could easily be made more efficient if needed.

[Devin Rousso]

Created attachment 396820 [details]
Patch

well one optimization would be to only look inside the `WI.TreeOutline` that actually changed :P

[EWS]

Committed r260375: <https://trac.webkit.org/changeset/260375>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396820 [details].

[Radar WebKit Bug Importer]

<rdar://problem/62066377>

[Blaze Burg]

Comment on attachment 396820 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:531
> +    _checkForEmptyFilterResults(treeOutline)

Naming nit:

I would not assume from the name that this updates the UI with the No Results placeholder. Nor can I understand what the parameter is. It replaces the per-outline function, but doesn't mention outline in the name.

I'd prefer _updateOutlineForEmptyFilterResultsIfNeeded(), maybe dropping IfNeeded, if needed. :)

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:572
> +        // All top level tree elements are hidden, so filtering hid everything. Show a message.

Nit on the original line: 'hid' is weird, I would use 'omitted'.