Bug 89517 - Web Inspector: ExtensionPanel.onSearch listener doesn't work
Summary: Web Inspector: ExtensionPanel.onSearch listener doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: https://code.google.com/p/chromium/is...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-19 15:18 PDT by Jan Keromnes
Modified: 2012-06-22 11:52 PDT (History)
13 users (show)

See Also:


Attachments
Patch (5.58 KB, patch)
2012-06-20 14:30 PDT, Jan Keromnes
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (560.33 KB, application/zip)
2012-06-20 15:24 PDT, WebKit Review Bot
no flags Details
Patch (5.99 KB, patch)
2012-06-21 12:04 PDT, Jan Keromnes
no flags Details | Formatted Diff | Diff
Patch (5.97 KB, patch)
2012-06-21 14:12 PDT, Jan Keromnes
no flags Details | Formatted Diff | Diff
Patch (6.31 KB, patch)
2012-06-22 00:06 PDT, Jan Keromnes
no flags Details | Formatted Diff | Diff
Patch (6.23 KB, patch)
2012-06-22 11:26 PDT, Jan Keromnes
janx: review+
janx: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Keromnes 2012-06-19 15:18:31 PDT
Cross-report from https://code.google.com/p/chromium/issues/detail?id=124800

Reported by kdzwinel, Apr 24, 2012
Chrome Version       : 18.0.1025.162 (Official Build 131933)
URLs (if applicable) :
Other browsers tested:

What steps will reproduce the problem?
1. Create an extension that adds new panel to Developer Tools:

chrome.devtools.panels.create("xxx", "img/icon_24.png", "xxx.html", function(panel) { //here goes code from point 2});

2. Set onSearch listener for given panel:

panel.onSearch.addListener(function(action, queryString) {console.log('eZTemplates onSearch ' + queryString);});

3.Load extension, open Developer Tools, navigate to new panel, enter text in search box (top right)

What is the expected result?
Our listener function should be fired.

What happens instead?
Listener function is never fired.


Please provide any additional information below. Attach a screenshot if
possible.

Bug appears on both Linux and Windows. Other listeners (onShow, onHidden) declared in the same way in the same chrome.devtools.panels.create callback work as expected.
Comment 1 Jan Keromnes 2012-06-20 14:30:08 PDT
Created attachment 148656 [details]
Patch
Comment 2 Jan Keromnes 2012-06-20 14:31:15 PDT
The bug is fixed but the test doesn't seem to work, I'm trying to figure out why.
Comment 3 WebKit Review Bot 2012-06-20 15:24:03 PDT
Comment on attachment 148656 [details]
Patch

Attachment 148656 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13009322

New failing tests:
inspector/extensions/extensions-panel.html
Comment 4 WebKit Review Bot 2012-06-20 15:24:12 PDT
Created attachment 148665 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 5 Jan Keromnes 2012-06-21 12:04:14 PDT
Created attachment 148857 [details]
Patch
Comment 6 Andrey Kosyakov 2012-06-21 13:53:05 PDT
Comment on attachment 148857 [details]
Patch

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

> LayoutTests/inspector/extensions/extensions-panel.html:87
> +            dumpObject({

dumpObject(Array.prototype.slice.call(arguments)) -- this will take care of all the arguments in case we accidentally add any.

> LayoutTests/inspector/extensions/extensions-panel.html:92
> +            if (callbackCount == 2)

== => ===

> LayoutTests/inspector/extensions/extensions-panel.html:116
> +    function performSearch()
> +    {
> +        evaluateOnFrontend("WebInspector.searchController._performSearch('hello', true, false, false); reply()");
> +    }
>      // The panel code is expected to report its size via InspectorTest.panelCallback()
> -    evaluateOnFrontend("InspectorTest.waitForPanel(reply);", nextTest);
> +    evaluateOnFrontend("InspectorTest.waitForPanel(reply);", performSearch);

This could be a separate test (in fact, show() could also be separate).
Comment 7 Jan Keromnes 2012-06-21 14:12:39 PDT
Created attachment 148886 [details]
Patch
Comment 8 Jan Keromnes 2012-06-21 14:20:30 PDT
(In reply to comment #6)
> dumpObject(Array.prototype.slice.call(arguments)) -- this will take care of all the arguments in case we accidentally add any.

Fixed.

> > LayoutTests/inspector/extensions/extensions-panel.html:92
> > +            if (callbackCount == 2)
> 
> == => ===

Also fixed.

> > LayoutTests/inspector/extensions/extensions-panel.html:116
> > +    function performSearch()
> > +    {
> > +        evaluateOnFrontend("WebInspector.searchController._performSearch('hello', true, false, false); reply()");
> > +    }
> >      // The panel code is expected to report its size via InspectorTest.panelCallback()
> > -    evaluateOnFrontend("InspectorTest.waitForPanel(reply);", nextTest);
> > +    evaluateOnFrontend("InspectorTest.waitForPanel(reply);", performSearch);
> 
> This could be a separate test (in fact, show() could also be separate).

I actually tried to place it in another test, but as _performSearch has no callback, the tests were flaky. Putting the tests back together was the only way I found to resolve that flakiness issue, and without creating several test panels.
Comment 9 Andrey Kosyakov 2012-06-21 18:09:21 PDT
Comment on attachment 148886 [details]
Patch

LGTM
Comment 10 Jan Keromnes 2012-06-22 00:06:30 PDT
Created attachment 148970 [details]
Patch
Comment 11 Jan Keromnes 2012-06-22 00:08:51 PDT
Better patch with separate tests.
Comment 12 Yury Semikhatsky 2012-06-22 00:20:52 PDT
Comment on attachment 148970 [details]
Patch

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

> Source/WebCore/inspector/front-end/ExtensionPanel.js:150
> +        WebInspector.extensionServer.notifySearchAction(this._panelName, WebInspector.extensionAPI.panels.SearchAction.CancelSearch);

Here and below, please use this.name instead of this._panelName as the latter is private for Panel.js file.
Comment 13 Jan Keromnes 2012-06-22 11:26:55 PDT
Created attachment 149071 [details]
Patch
Comment 14 Jan Keromnes 2012-06-22 11:28:05 PDT
(In reply to comment #12)
> > Source/WebCore/inspector/front-end/ExtensionPanel.js:150
> > +        WebInspector.extensionServer.notifySearchAction(this._panelName, WebInspector.extensionAPI.panels.SearchAction.CancelSearch);
> 
> Here and below, please use this.name instead of this._panelName as the latter is private for Panel.js file.

Fixed.
Comment 15 Andrey Kosyakov 2012-06-22 11:48:12 PDT
Committed r121046: <http://trac.webkit.org/changeset/121046>