Bug 89517 – Web Inspector: ExtensionPanel.onSearch listener doesn't work

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Jan Keromnes]

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.

[Jan Keromnes]

Created attachment 148656 [details]
Patch

[Jan Keromnes]

The bug is fixed but the test doesn't seem to work, I'm trying to figure out why.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Jan Keromnes]

Created attachment 148857 [details]
Patch

[Andrey Kosyakov]

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).

[Jan Keromnes]

Created attachment 148886 [details]
Patch

[Jan Keromnes]

(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.

[Andrey Kosyakov]

Comment on attachment 148886 [details]
Patch

LGTM

[Jan Keromnes]

Created attachment 148970 [details]
Patch

[Jan Keromnes]

Better patch with separate tests.

[Yury Semikhatsky]

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.

[Jan Keromnes]

Created attachment 149071 [details]
Patch

[Jan Keromnes]

(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.

[Andrey Kosyakov]

Committed r121046: <http://trac.webkit.org/changeset/121046>