Bug 167072 – Web Inspector: can't jump from Search Tab result to see resource in other tabs (Resource, Debugger, Network)

• Component: Web Inspector
• Status: RESOLVED FIXED

[Blaze Burg]

I suggest we add a jump icon after the result, and clicking it jumps to the default tab for the resource. Control-clicking shows a shortcut menu with options to show in each supportedin  tab (Show in Resources Tab, Show in Debugger Tab, etc). There is currently no way to directly jump to show the resource in a non-Search tab.

[Radar WebKit Bug Importer]

<rdar://problem/30033496>

[Devin Rousso]

Created attachment 300165 [details]
Patch

[Matt Baker]

Comment on attachment 300165 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/TabBrowser.js:123
> +                continue;

This prevents the Search tab from being shown when performing a search while another tab is showing:

1. Goto daringfireball.net
2. Show Network tab
3. Enter "fire" in search field, press Enter
Expected:
    => Taken to search tab, first result selected
Actual:
   => Network tab content view changes, tab does not (see trace).

[Log] Trace
    bestTabContentViewForRepresentedObject (TabBrowser.js:123)
    tabContentViewForRepresentedObject (Main.js:1138)
    showRepresentedObject (Main.js:1157)
    showSourceCode (Main.js:1203)
    showOriginalOrFormattedSourceCodeTextRange (Main.js:1224)
    _treeSelectionDidChange (SearchSidebarPanel.js:371)
    dispatch (Object.js:170)
    dispatchEventToListeners (Object.js:177)
    select (TreeElement.js:507)
    _contentTreeOutlineDidFocus (NavigationSidebarPanel.js:667)
    focus
    select (TreeElement.js:476)
    revealAndSelect (TreeElement.js:516)
    (anonymous function) (SearchSidebarPanel.js:154)
    (anonymous function)
    forEachMatch (SearchSidebarPanel.js:120)
    resourceCallback (SearchSidebarPanel.js:149)
    resourceCallback
    _dispatchResponseToCallback (Connection.js:146)
    _dispatchResponse (Connection.js:116)
    dispatch (Connection.js:69)
    dispatch (InspectorBackend.js:154)
    dispatchNextQueuedMessageFromBackend (MessageDispatcher.js:42)

[Devin Rousso]

Created attachment 300283 [details]
Patch

[Devin Rousso]

Created attachment 300302 [details]
Patch

[Build Bot]

Comment on attachment 300302 [details]
Patch

Attachment 300302 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2985322

New failing tests:
css3/filters/backdrop/dynamic-with-clip-path.html

[Build Bot]

Created attachment 300322 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Timothy Hatcher]

Comment on attachment 300302 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/SearchResultTreeElement.js:93
> +    ondblclick(event)
> +    {
> +        const options = {ignoreSearchTab: true};
> +        if (this.representedObject instanceof WebInspector.DOMSearchMatchObject)
> +            WebInspector.showMainFrameDOMTree(this.representedObject.domNode, options);
> +        else if (this.representedObject instanceof WebInspector.SourceCodeSearchMatchObject)
> +            WebInspector.showOriginalOrFormattedSourceCodeTextRange(this.representedObject.sourceCodeTextRange, options);
> +    }

This code makes more sense in a controller class, like we do for single click (selected) events.

> Source/WebInspectorUI/UserInterface/Views/TabBrowser.js:123
> +            if (options.ignoreSearchTab && tabContentView instanceof WebInspector.SearchTabContentView)
> +                continue;

Maybe instead of ignoreSearchTab, ignoreCurrentTab? Though I think ignoreSearchTab is useful too. So this is fine as-is.

[Devin Rousso]

Comment on attachment 300302 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/SearchResultTreeElement.js:93
>> +    }
> 
> This code makes more sense in a controller class, like we do for single click (selected) events.

So I've seen code go both ways for something like this (e.g. contextmenu items for "Reveal in Resources Tab").  Are you suggesting that I propagate this event to the controller (via some event listener or delegate), which is SearchSidebarPanel in this case?  To me, that seems more complex that this solution.

[Devin Rousso]

Created attachment 300356 [details]
Patch

[Timothy Hatcher]

Comment on attachment 300356 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/TreeElement.js:318
> +        element.treeElement.dispatchEventToListeners(WebInspector.TreeElement.Event.DoubleClick);

This should prevent the double click to expand code that is above, if the event prevents default, like ondblclick does.

[Devin Rousso]

Created attachment 300463 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 300463 [details]
Patch

Clearing flags on attachment: 300463

Committed r211608: <http://trac.webkit.org/changeset/211608>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.