Bug 124544 - Web Inspector: Update WebInspectorUI to use the new "nodeIds" parameter for DOM.performSearch
Summary: Web Inspector: Update WebInspectorUI to use the new "nodeIds" parameter for D...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexandru Chiculita
Depends on:
Reported: 2013-11-18 15:47 PST by Alexandru Chiculita
Modified: 2013-11-18 17:11 PST (History)
5 users (show)

See Also:

Patch V1 (5.55 KB, patch)
2013-11-18 15:50 PST, Alexandru Chiculita
joepeck: review+
Details | Formatted Diff | Diff
Patch for landing (5.70 KB, patch)
2013-11-18 16:39 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Chiculita 2013-11-18 15:47:43 PST
The search should be limited to just the view the user has open. In 124390 a new parameter has been added on the protocol to help with that. Make sure we are using the new parameter in the FrameDOMTreeContentView and ContentFlowDOMTreeContentView. This way the search results will be limited to just the elements visible in the current view.
Comment 1 Alexandru Chiculita 2013-11-18 15:50:30 PST
Created attachment 217245 [details]
Patch V1
Comment 2 Joseph Pecoraro 2013-11-18 16:28:07 PST
Comment on attachment 217245 [details]
Patch V1

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

> Source/WebInspectorUI/UserInterface/DOMTreeContentView.js:215
> -        DOMAgent.performSearch(query, searchResultsReady.bind(this));
> +        function contextNodesReady(nodeIds)
> +        {
> +            DOMAgent.performSearch(query, nodeIds, searchResultsReady.bind(this));
> +        }

NOTE TO SELF: Adding an extra optional parameter like this does not break iOS 6 and 7 compatibility. So this is good.

This might even be a good comment to throw into the ChangeLog.

> Source/WebInspectorUI/UserInterface/DOMTreeContentView.js:224
> +        callback();

Nit: I would prefer to see callback(undefined) to more explicitly show we're passing undefined to performSearch.
Comment 3 Alexandru Chiculita 2013-11-18 16:39:31 PST
Created attachment 217252 [details]
Patch for landing

Thanks Joe!
Comment 4 WebKit Commit Bot 2013-11-18 17:11:18 PST
Comment on attachment 217252 [details]
Patch for landing

Clearing flags on attachment: 217252

Committed r159475: <http://trac.webkit.org/changeset/159475>
Comment 5 WebKit Commit Bot 2013-11-18 17:11:20 PST
All reviewed patches have been landed.  Closing bug.