[Webkit-unassigned] [Bug 88473] Web Inspector: Implement ExtensionPanel.show() method

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88473





--- Comment #4 from Jan Keromnes <janx at chromium.org>  2012-06-07 11:52:27 PST ---
(In reply to comment #2)

Thanks for your input on this Andrey!

> We're somewhat wary of the extensions switching focus unexpectedly for the user. So it would be good idea to add a check that only allows panel.show() to be called in response to a user action (I can only think of one so far -- open resource handler). So the idea is to set a flag upon invoking the handler, clearing it upon exit from the handle and checking it in show(). Unfortunately, with server-to-client callback being async, this would have to be done on the client side -- but it's still better than nothing :)

I believe unexpected focus-grabbing extensions would only harm themselves, because users would remove them and dissuade other users from installing it. But if you think it's important to prevent that, I will gladly implement a check for it.

What if the extension panel is a debugger and the runtime hits a breakpoint?

Another solution would be to make handlers (like the open resource handler) return a panel ID to bring up, but I'm not sure if it's a good idea.

> 
> > Source/WebCore/inspector/front-end/ExtensionAPI.js:399
> > +    },
> > +    show: function()
> 
> please add empty line before show.

Will do.

> > Source/WebCore/inspector/front-end/ExtensionServer.js:210
> > +        WebInspector.showPanel(message.id);
> 
> Please add a check for id to be the extension panel (i.e. id in this._clientObjects). It wouldn't be possible to pass incorrect panel id through an API call, but we're trying to also protect from direct messages sent to the extension server. Also very unlikely, but just in case :)

The method `WebInspector.showPanel()` in `inspector.js:835` is already protected against wild input, so I think it is unnecessary to duplicate that protection.

> 
> > LayoutTests/inspector/extensions/extensions-panel.html:74
> > +        panel.onSearch.addListener(function(action, query) {
> > +            output("Panel searched");
> > +        });
> 
> This does not seem to have any effect on the test output, so please remove it for now.

Will remove it for now. I am planning on adding a failing test in another patch for the `onSearch()` callback not being called in extension panels (see issue https://code.google.com/p/chromium/issues/detail?id=124800).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.