[Webkit-unassigned] [Bug 106811] Web Inspector: Add iframe option to inspectedWindow.eval() extension API
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jan 16 10:08:59 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=106811
--- Comment #7 from johnjbarton <johnjbarton at chromium.org> 2013-01-16 10:10:43 PST ---
(In reply to comment #6)
> (From update of attachment 182649 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182649&action=review
>
> > Source/WebCore/inspector/front-end/ExtensionServer.js:801
> > + } else if (options.frame && typeof options.frame === "object" && options.frame.url && options.frame.securityOrigin) {
>
> can we support options.frame and options.useContentScriptContext at the same time? I can imagine this may be useful.
Yes, good idea.
>
> > Source/WebCore/inspector/front-end/ExtensionServer.js:810
> > + context = contextLists[i].contextBySecurityOrigin(options.frame.securityOrigin);
>
> So this will allow an extension to evaluate in content script of other extensions. This may be a bit more than we would like to allow now. How about just using frame.url and handling useContentScriptContext.
It allows a devtools extension to evaluate in the context of another browser extension. Two browser extensions are 'peers': they have equivalent possible permissions and powers. Allowing browser extensions to operate on other browser extensions could lead to confusing interference.
Devtools extensions are not peers of browser extensions: they are explicitly -- by the specific user opening devtools -- granted extra power -- eg the ability to eval() directly into the context of web page. Shouldn't devtools extensions should be able to eval() into browser extensions just like devtools can?
>
> > Source/WebCore/inspector/front-end/ExtensionServer.js:816
> > + callback(errorMessage);
>
> this should not be necessary, if you return an error status, the caller will invoke callback.
>
> > Source/WebCore/inspector/front-end/ExtensionServer.js:817
> > + return this._status.E_FAILED(errorMessage);
>
> I think E_NOTFOUND is more appropriate here, E_FAILED is too generic.
Actually the opposite was my experience: I started with E_NOTFOUND but I could not figure out what to do with an error message like 'Not found "http://localhost:9696"'. I realize that E_NOTFOUND makes better documentation in the source code but the resulting error message is not as good for the extension author.
>
> > LayoutTests/inspector/extensions/extensions-eval-expected.txt:19
> > +log: Extension server error: Operation failed: No context with URL: "file:///work/chromium/src/third_party/WebKit/LayoutTests/inspector/extensions/resources/extensions-frame-eval.html" and origin "bogus"
>
> Well.. That's not going to ever pass on the bots :)
It would if everyone switched to my file system layout ;-) Too much cut and paste I can see.
>
> > LayoutTests/inspector/extensions/extensions-eval.html:49
> > +function extension_testZEvalInIFrame(nextTest) {
>
> Z?
For some reason the test issues tests.sort() and I wanted this test to run last.
>
> > LayoutTests/inspector/extensions/extensions-eval.html:50
> > + var iframeSrc = 'resources/extensions-frame-eval.html';
>
> I think a good test would be to try something cross-origin -- i.e. move it under http tests and use http://127.0.0.1:8000/ as on origin.
Sure.
Let me know about the other issues here and I will resubmit the patch.
Thanks!
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list