Bug 167072

Summary: Web Inspector: can't jump from Search Tab result to see resource in other tabs (Resource, Debugger, Network)
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch
timothy: review+
Patch none

Description BJ Burg 2017-01-15 13:59:02 PST
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.
Comment 1 Radar WebKit Bug Importer 2017-01-15 13:59:18 PST
<rdar://problem/30033496>
Comment 2 Devin Rousso 2017-01-30 17:23:12 PST
Created attachment 300165 [details]
Patch
Comment 3 Matt Baker 2017-01-31 15:27:01 PST
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)
Comment 4 Devin Rousso 2017-01-31 16:47:34 PST
Created attachment 300283 [details]
Patch
Comment 5 Devin Rousso 2017-01-31 22:27:12 PST
Created attachment 300302 [details]
Patch
Comment 6 Build Bot 2017-02-01 05:03:57 PST
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
Comment 7 Build Bot 2017-02-01 05:04:00 PST
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
Comment 8 Timothy Hatcher 2017-02-01 09:12:36 PST
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.
Comment 9 Devin Rousso 2017-02-01 12:50:24 PST
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.
Comment 10 Devin Rousso 2017-02-01 14:51:38 PST
Created attachment 300356 [details]
Patch
Comment 11 Timothy Hatcher 2017-02-02 07:29:36 PST
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.
Comment 12 Devin Rousso 2017-02-02 16:10:26 PST
Created attachment 300463 [details]
Patch
Comment 13 WebKit Commit Bot 2017-02-02 16:24:15 PST
Comment on attachment 300463 [details]
Patch

Clearing flags on attachment: 300463

Committed r211608: <http://trac.webkit.org/changeset/211608>
Comment 14 WebKit Commit Bot 2017-02-02 16:24:21 PST
All reviewed patches have been landed.  Closing bug.