1211859 - [tracking] Implement an initial subset of WebExtensions devtools APIs

Status: RESOLVED FIXED

(WebExtensions :: Developer Tools, defect, P2)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

https://developer.chrome.com/extensions/devtools
https://developer.chrome.com/extensions/devtools_inspectedWindow
https://developer.chrome.com/extensions/devtools_panels
https://developer.chrome.com/extensions/devtools_network

devtools API json description: 
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/extensions/api/devtools/

API is listed on https://wiki.mozilla.org/WebExtensions#List_of_APIs_we_will_likely_support_in_the_future

devtools API helps porting Chrome Devtools Addons (e.g. Backbone Debugger, Angular Batarang, etc.)

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: prototype-open-extension-devtools-API.patch

This patch contains a prototype (or proof of concept) of the changes needed to implement a good chunk of the devtools API (just the amount needed to be able to successfully execute a real world addon: https://github.com/Maluen/Backbone-Debugger/ , actually a fork of the original addon, in which I fixed some minor issues that didn't make sense to fix in the API implementation, I'm going to attach a link to the forked version asap, once I've pushed it on github).

The following are the components already in the prototype:

- "browser/components/extensions/ext-devtools.js" is where the new open extension devtools API are defined
- "devtools/server/actors/webextension-devtools-api.js" is where a new Remote Debugger Server actor is defined (it is used to expose the helpers needed by inspectedWindow.eval and inspectedWindoe.reload)
- "browser/components/extensions/test/browser/browser_ext_devtools_page.js" is a prelimary test case

Notes on implementation strategies, issues and workarounds:

- there is a devtools page instance per toolbox (it will be created on toolbox-ready and unloaded on toolbox-destroy)

- the devtools page can register new devtools panels using the devtools.panels.create API, this is in contrast to the current API currently exposed by Firefox Devtools which exposes a gDevTools.registerTool method which registers a tool globally and automatically build the tool on every new toolbox created (to workaround this difference and prevent the devtools.panels.create to register globally the same panel more than once, an unique id is auto-generated for the devtools, based on the extension id and the panel url)    

- the custom RDP actor helps to open the API to remote targets in the future (but it is not enough mostly because a devtools addon usually uses other APIs to fully implement their features, e.g. content-scripts and the tabs onUpdate API)

- in the current prototype the inspectedWindow.eval currently contains two implementations: using the Remote Debugging Protocol or using the unsafe CPOW, to be able to check the differences of the two alternatives (because by testing the APIs on the Backbone Debugger it seems that the latency of manually exchange the needed messages, e.g. RDP or messagemanager request/reply time, is too long and the addon doesn't receive the expected message when it should)

- the inspectedWindow.reload can optionally execute an injected script in the target window context, the custom RDP actor use the DocumentManager singleton to subscribe the script to be executed on document-start for the target window 

Not yet implemented features:

- privileged APIs (e.g. tabs) should not be exposed to the devtools page and panel contexts
- the devtools.inspectedWindow.eval can optionally execute in the context of the content script or on a defined frameURL
- the devtools.inspectedWindow.getResources is currently just a shim
- the devtools.panels.create should pass a Panel object to the callback
- the devtools elements and sources createSidebarPane is not implemented 
- the devtools.network is not implemented

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: webext-devtools-api sketched diagram

I'm attaching a small sketched diagram of the components in the prototype

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: webext-devtools-api sketched diagram

I'm attaching the full diagram (in the previous image I have exported an incomplete image by mistake)

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Drive-by review: The call to "gDevTools.registerTool" on line 132 of ext-devtools.js is missing the "icon: icon" line to show the icon…  (Found while implementing https://github.com/bwinton/whimsy/commit/e80f9fef516b858892732e8b6d50f44219bc0733 :)

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Blake Winton (:bwinton) (:☕️) from comment #4)
> Drive-by review: The call to "gDevTools.registerTool" on line 132 of
> ext-devtools.js is missing the "icon: icon" line to show the icon…  (Found
> while implementing
> https://github.com/bwinton/whimsy/commit/
> e80f9fef516b858892732e8b6d50f44219bc0733 :)

Thank a lot Blake: you tested the patch fast as light.

I've fixed it locally in the mean time (to be sure to not forget it)
(but I'm waiting the preliminary feedback before updating the attached patch)

Comment 6

[Alexandre Poirot [:ochameau]]

Comment on attachment 8670326 [details] [diff] [review]
prototype-open-extension-devtools-API.patch

Review of attachment 8670326 [details] [diff] [review]:
-----------------------------------------------------------------

I just started looking at the actor, and I think you can use existing method on console and tab actors.

::: browser/components/extensions/ext-devtools.js
@@ +213,5 @@
> +      target.client.request({
> +        to: target.form.webextensionDevtoolsActor,
> +        type: "inspectedWindowEval",
> +        text, options
> +      }, (res) => {

Same thing here, if you fetch a client for the webconsole actor, you have this existing request:
http://mxr.mozilla.org/mozilla-central/source/devtools/shared/webconsole/client.js#212

I think you can fetch it from the target thanks to this:
http://mxr.mozilla.org/mozilla-central/source/devtools/client/framework/target.js#461
So:
  target.activeConsole.evaluateJS(...);

@@ +287,5 @@
> +        }
> +
> +        eval(text, options, cb);
> +      },
> +      reload(options) {

You don't need to introduce this new request.
TabActor already support a reload request.

http://mxr.mozilla.org/mozilla-central/source/devtools/shared/client/main.js#1243
  target.activeTab.reload();
  target.activeTab.reload({force:true}); // to bypass cache

Comment 7

[Dylan Barrell]

Would it be possible to extend the devtools API to allow extensions to use the highlighting functionality and inspect functionality that is built into the dev tools. For example, right now we have to do this:

exports.highlightDOMElement = function(toolbox, selectorArray) {
	toolbox.loadTool('inspector').then(function() {
		let {
			walker
		} = toolbox.getPanel('inspector');
		walker.querySelector(walker.rootNode, selectorArray[0])
			.then(function(result) {
				var depth = 1,
					contentDocument;
				while (result.nodeName === 'IFRAME' && selectorArray.length > depth) {
					contentDocument = result.rawNode().contentDocument;
					result = walker.frontForRawNode(contentDocument.querySelector(selectorArray[depth]));
					depth += 1;
				}
				toolbox.highlighterUtils.highlightNodeFront(result);
			});
	});
};

Another good enhancement to this would be a way to get access to a node front that is inside an iframe without having to access the rawNode as in the above code.

Comment 8

[Brian Grinstead [:bgrins]]

Comment on attachment 8670326 [details] [diff] [review]
prototype-open-extension-devtools-API.patch

Review of attachment 8670326 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/extensions/ext-devtools.js
@@ +213,5 @@
> +      target.client.request({
> +        to: target.form.webextensionDevtoolsActor,
> +        type: "inspectedWindowEval",
> +        text, options
> +      }, (res) => {

Note: `target.activeConsole.evaluateJSAsync` plays nicer when the thread is paused and will fall back to evaluateJS if the server doesn't support it (pre-37)

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I can't cleanly apply the patch. What am I doing wrong?
Honza

$ patch -p1 --dry-run < 0003-Bug-1211859-add-open-extension-devtools-API-and-de
vt.patch
patching file browser/components/extensions/ext-devtools.js
patching file browser/components/extensions/jar.mn
patching file browser/components/extensions/test/browser/browser.ini
Hunk #1 succeeded at 11 with fuzz 1 (offset 2 lines).
patching file browser/components/extensions/test/browser/browser_ext_devtools_pa
ge.js
patching file browser/components/extensions/test/browser/head.js
Hunk #1 succeeded at 1 with fuzz 1.
patching file browser/components/nsBrowserGlue.js
Hunk #1 FAILED at 583.
Hunk #2 succeeded at 631 with fuzz 1 (offset 4 lines).
Hunk #3 succeeded at 807 (offset 5 lines).
Hunk #4 succeeded at 2039 (offset 5 lines).
Hunk #5 succeeded at 2201 (offset 5 lines).
1 out of 5 hunks FAILED -- saving rejects to file browser/components/nsBrowserGl
ue.js.rej
patching file devtools/server/actors/moz.build
patching file devtools/server/actors/webextension-devtools-api.js
patching file devtools/server/main.js
patching file toolkit/components/extensions/ExtensionContent.jsm

Comment 10

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: prototype-open-extension-devtools-API-v2.patch

This new version of the previous patch contains the following changes based on the initial feedback:

- fixed missing icon in registered devtool panel
- uses tab and console actor instead of registering a new actor
- adds to webconsole actor's onEvaluateJSAsync two new options (unsafeDereference, used to request thea plain version of the result instead of the result grip, and runOnNextWindowReady, used to defer the evaluation to the next window-ready event)
- removes the temporary webextension.devtool.evalType preference, used to compare the timing of the local unsafeCPOW eval and the RDP based eval

The reason why I initially registered a minimal actor was to evaluate the differences with the existent actors (e.g. initially I was using the webconsole actor as it is but without the above changes I had to change the evaluated javascript code on the fly to convert the result into JSON before it is turned into a grip to be returned to the client).

The above changes to the webconsole actor cover the following new feature:
- get the plain result back from the actor
- schedule an evaluateJS on the next window-ready event

But to reach parity on the devtools.inspectedWindow.eval/reload, we will need to add the following:

- inspectedWindow.reload can optionally customize the userAgent
- inspectedWindow.eval can optionally:
  - frameURL: execute into iframes with a defined "frameURL"
  - useContentScriptContext: execute in the extension content script context
    (so we need be able to get the content script context in the actors
    somewhere, in the webconsole actor or in another actor) 
  - contextSecurityOrigin: in the content script which match a defined origin.

we can defer some of this feature in followups, but we need to take them into account.

Comment 11

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Jan Honza Odvarko [:Honza] from comment #9)
> I can't cleanly apply the patch. What am I doing wrong?
> Honza
> 
> $ patch -p1 --dry-run <
> 0003-Bug-1211859-add-open-extension-devtools-API-and-de
> vt.patch
> patching file browser/components/extensions/ext-devtools.js
> patching file browser/components/extensions/jar.mn
> patching file browser/components/extensions/test/browser/browser.ini
> Hunk #1 succeeded at 11 with fuzz 1 (offset 2 lines).
> patching file
> browser/components/extensions/test/browser/browser_ext_devtools_pa
> ge.js
> patching file browser/components/extensions/test/browser/head.js
> Hunk #1 succeeded at 1 with fuzz 1.
> patching file browser/components/nsBrowserGlue.js
> Hunk #1 FAILED at 583.
> Hunk #2 succeeded at 631 with fuzz 1 (offset 4 lines).
> Hunk #3 succeeded at 807 (offset 5 lines).
> Hunk #4 succeeded at 2039 (offset 5 lines).
> Hunk #5 succeeded at 2201 (offset 5 lines).
> 1 out of 5 hunks FAILED -- saving rejects to file
> browser/components/nsBrowserGl
> ue.js.rej
> patching file devtools/server/actors/moz.build
> patching file devtools/server/actors/webextension-devtools-api.js
> patching file devtools/server/main.js
> patching file toolkit/components/extensions/ExtensionContent.jsm

I've recently started to use git-cinnabar and moz-git-tools, the patches converted from git to hg format do not have any reference to the hg commit they relate to, this probably generate more conflicts than expected if the patch cannot be applied cleanly anymore (due to changes on the mozilla-central and because it is impossible to calculate the 3-way merge)

I've just attached an updated patch (which I rebased locally on a recent mozilla-central), it should apply without conflicts, let me know if it doesn't (I'm still evaluating my "git-cinnabar"-based setup and workflow)

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Luca Greco from comment #10)
> - inspectedWindow.reload can optionally customize the userAgent

I would suggest leaving this as a follow up, considering we don't yet have it from our native tools either.

Bug 1137681 includes a WIP patch of how we might offer this feature.

Comment 13

[Alexandre Poirot [:ochameau]]

(In reply to Luca Greco from comment #10)
> Created attachment 8672616 [details] [diff] [review]
>
> The reason why I initially registered a minimal actor was to evaluate the
> differences with the existent actors (e.g. initially I was using the
> webconsole actor as it is but without the above changes I had to change the
> evaluated javascript code on the fly to convert the result into JSON before
> it is turned into a grip to be returned to the client).

Our protocol is using JSON, so there shouldn't be much convertion.

> 
> The above changes to the webconsole actor cover the following new feature:
> - get the plain result back from the actor

This API has very precise constrains different from evaluateJSAsync:
"The expression must evaluate to a JSON-compliant object, otherwise an exception is thrown."
The whole purpose of webconsole API is to support *any* object, so it feel hacky to introduce this plainObject attribute that is going to fail unexpectedly, I imagine in middle of transport layer, when you are going to evaluate a non-JSON-able object.

> - schedule an evaluateJS on the next window-ready event

Hum. It looks also hacky. Can we listen for `navigate` event on the target on the client side before evaluating the script? Or does the script have to be evaluated before page load?

> - inspectedWindow.eval can optionally:
>   - frameURL: execute into iframes with a defined "frameURL"
>   - useContentScriptContext: execute in the extension content script context
>     (so we need be able to get the content script context in the actors
>     somewhere, in the webconsole actor or in another actor) 
>   - contextSecurityOrigin: in the content script which match a defined
> origin.

Hum. In order to support all these contexts: iframe and content script, we would need to have a Console actor for the context we want to evaluate to. 
For iframe, it would mean switching the whole tab actor context to a given iframe, that doesn't sound reasonable. But may be we can spawn a new instance for a given frame??
For content scripts, today there is no way to debug them, but we do want that! And it means having debugger and console actors up and running for these contexts. So you may depend on that.

Or. You come up with a new kind of actor specific to devtools API, that would accept targets and easily apply whatever action you need to various contexts, but it may be complex to support e10s.

I can easily imagine there is even more alternatives than just these two...

Comment 14

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Alexandre Poirot [:ochameau] from comment #13)
> (In reply to Luca Greco from comment #10)
> > Created attachment 8672616 [details] [diff] [review]
> >
> > The reason why I initially registered a minimal actor was to evaluate the
> > differences with the existent actors (e.g. initially I was using the
> > webconsole actor as it is but without the above changes I had to change the
> > evaluated javascript code on the fly to convert the result into JSON before
> > it is turned into a grip to be returned to the client).
> 
> Our protocol is using JSON, so there shouldn't be much convertion.

sorry, I wasn't very clear about it: I meant that to get the plain result from the webconsole evaluateJS method I had to stringify the object in the evaluated javascript code, before it is turned into a grip to be returned to the client (it was just an hacky solution that I initially used to get the plain object from the webconsole actor during the first prototype of this patch, before looking into more complete alternatives).

> 
> > 
> > The above changes to the webconsole actor cover the following new feature:
> > - get the plain result back from the actor
> 
> This API has very precise constrains different from evaluateJSAsync:
> "The expression must evaluate to a JSON-compliant object, otherwise an
> exception is thrown."
> The whole purpose of webconsole API is to support *any* object, so it feel
> hacky to introduce this plainObject attribute that is going to fail
> unexpectedly, I imagine in middle of transport layer, when you are going to
> evaluate a non-JSON-able object.

I agree, and this is something we should prevents even if we're creating a new specific actor instead of changing the webconsole actor.

to prevent the fail to happen in the middle of the transport layer, how about stringify the object before sending it on the wire and catch any JSON.stringify exception to return a proper error message? 

> 
> > - schedule an evaluateJS on the next window-ready event
> 
> Hum. It looks also hacky. Can we listen for `navigate` event on the target
> on the client side before evaluating the script? Or does the script have to
> be evaluated before page load?

I would prefer to do it all on the remote side, because even if it will be restricted to
local tabs initially, I'm trying to do not do anything that would prevent it to work
on a remote target in the future.

and besides that, this feature needs to inject the script on the very start of the document loading (e.g. like "document_start" for content scripts) and I fear that the script can randomly be injected at the wrong time because of changes into the latency between receiving the "window is ready" event on the client side and sending/receiving the RDP request to inject the script.

> 
> > - inspectedWindow.eval can optionally:
> >   - frameURL: execute into iframes with a defined "frameURL"
> >   - useContentScriptContext: execute in the extension content script context
> >     (so we need be able to get the content script context in the actors
> >     somewhere, in the webconsole actor or in another actor) 
> >   - contextSecurityOrigin: in the content script which match a defined
> > origin.
> 
> Hum. In order to support all these contexts: iframe and content script, we
> would need to have a Console actor for the context we want to evaluate to. 
> For iframe, it would mean switching the whole tab actor context to a given
> iframe, that doesn't sound reasonable. But may be we can spawn a new
> instance for a given frame??
> For content scripts, today there is no way to debug them, but we do want
> that! And it means having debugger and console actors up and running for
> these contexts. So you may depend on that.
> 
> Or. You come up with a new kind of actor specific to devtools API, that
> would accept targets and easily apply whatever action you need to various
> contexts, but it may be complex to support e10s.
> 
> I can easily imagine there is even more alternatives than just these two...

That is the reason I described all the features, even the missing features, because before changing an existing actor I wanted to be sure that, taking into account both the current and the next features, we're putting the changes into the right place.

Based on your feedback, I would like to make the following proposal:

- move the evaluation helpers from webconsole into a shared module (e.g. the evalWithDebugger method, which currently uses "this", the webconsole actor, just to get the current connection, the debugger instance and the evalWindow)
- change webconsole to get the evaluation helper from the above shared module
- add a new RDP actor for the webextension devtools API which initially just reuses the evalWithDebugger method to recreate the initial set of feature and can be expanded to provide the other features without changing too much in the webconsole actor

is this a reasonable strategy?

Comment 15

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Sorry this is taking me so long. I guarantee I'll look at it tomorrow.

Comment 16

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8672616 [details] [diff] [review]
prototype-open-extension-devtools-API-v2.patch

Review of attachment 8672616 [details] [diff] [review]:
-----------------------------------------------------------------

It sounds like most of the difficult issues for this patch are devtools-related. I made some suggestions for the webextension code, but basically it looks good.

::: browser/components/extensions/ext-devtools.js
@@ +50,5 @@
> +  buildForToolbox(toolbox) {
> +    if (this.targetContextMap.has(toolbox.target)) {
> +      return;
> +    }
> +    let webNav = Services.appShell.createWindowlessBrowser(false);

We should probably factor out this code into a method on the Extension. Then ext-backgroundPage and ext-devtools could share it.

@@ +85,5 @@
> +  }
> +}
> +
> +gDevTools.on("toolbox-ready", (evt, toolbox) => {
> +  for (let entry of devtoolsPageMap.entries()) {

You can do |for (let [extension, devtoolsPage] of devtoolsPageMap) {|.

@@ +169,5 @@
> +    browser.setAttribute("style", "width: 100%; height: 100%;");
> +    document.body.setAttribute("style", "margin: 0; padding: 0;")
> +    document.body.appendChild(browser);
> +
> +    let url = this.extension.baseURI.resolve(this.url);

This patch could benefit from bug 1214952.

@@ +220,5 @@
> +    context.windowId = windowId;
> +    context.tabId = tabId;
> +  } else {
> +    // TODO: windowID and tabId for remote tabs needs to get tab manager info
> +    // from the TabManager on the remote side

Is this for when we're debugging the browser itself? I don't understand what "local" and "remote" mean in this context. For e10s, the code for isLocalTab should work fine.

Comment 17

[Brian Grinstead [:bgrins]]

Comment on attachment 8672616 [details] [diff] [review]
prototype-open-extension-devtools-API-v2.patch

Review of attachment 8672616 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/server/actors/webconsole.js
@@ +816,3 @@
>  
> +    if (aRequest.runOnNextWindowReady) {
> +      events.once(this.parentActor, "window-ready", evaluate);

I guess the target doesn't get a window-ready, but I wonder if we could instead use the 'navigate' event on the target client-side and make a normal call to evaluateJSAsync after that.  note: I haven't tested this and I'm not sure of the distinction between the two events off the top of my head, although the webconsole frontend uses this event to detect reloads client side and it seems like it might fit in better to manage that state client side.

@@ +887,5 @@
> +        undefined : result;
> +      value = value && (typeof value.unsafeDereference == "function") ?
> +        value.unsafeDereference() : value;
> +
> +      ret.plainResult = value;

Looks like this is returning a reference to an object in the response packet for this call.  That probably won't work how you want on a remote connection (unless if the object happens to be a string or something).  What specifically are you trying to do with this option?

Comment 18

[Alexandre Poirot [:ochameau]]

Comment on attachment 8672616 [details] [diff] [review]
prototype-open-extension-devtools-API-v2.patch

Review of attachment 8672616 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Luca Greco from comment #14)
> That is the reason I described all the features, even the missing features,
> because before changing an existing actor I wanted to be sure that, taking
> into account both the current and the next features, we're putting the
> changes into the right place.

I'm having a second thought about that. I think you got it right in your first attempt.
It now sounds like an hassle to reuse the console.
We are trying to reuseit just for the sake of reusing some code and pattern.
At the end, there is many things you don't want nor need: evaluating via a Debugger object, supporting $, console and various other stuff in global scope. It looks like you just want window.eval or do we need to expose some specifics to the piece of JS we want to evaluate?

Then, I don't know what to do about context selection (iframe, content scripts).
We also have an existing pattern, where the TabActor is having a percise target that we can change.
Childs of the tab actors, like your new custom actors would query tabActor.window or tabActor.docShell to get the current context of the tab actor. Today this context can be the top level document or any of its frame. We could imagine extending that to content scripts, but that would be hard for two reasons:
* content scripts are not document and various childs of the tab actor expect documents (like inspector)
* this context is changed globaly for all the childs
It looks like reusing this would also be an hassle...
I wish we already had support for content script debugging to have a better picture of this!

> is this a reasonable strategy?

At the end, I think your custom actor may be just better. I would like to know how you would tackle iframes/content scripts.

::: browser/components/extensions/ext-devtools.js
@@ +21,5 @@
> +    this.url = url;
> +    this.extension = extension;
> +
> +    // Map[target -> ExtensionPage]
> +    this.targetContextMap = new Map();

What is the reasoning behind using target as key instead of toolbox?

@@ +66,5 @@
> +
> +    let window = webNav.document.defaultView;
> +    context.contentWindow = window;
> +    context.webNav = webNav;
> +    context.docShell = docShell;

Same question about context here.

@@ +131,5 @@
> +        url: "about:blank",
> +        icon: icon ? this.extension.baseURI.resolve(icon) : null,
> +        label: title,
> +        invertIconForLightTheme: true,
> +        isTargetSupported: target => target.isLocalTab,

Not important for the first iteration, but I'm wondering...
I imagine that's the only thing that prevents it from running with a remote target?
it looks like if you land the actor tweaks, that should work as-is with remotes.
But I'm wondering if the webextensions stuff would even work for toolbox within webide??
I imagine gDevtools.on("toolbox-ready" is also firing for WebIDE toolboxes.

@@ +178,5 @@
> +      uri: Services.io.newURI(url, null, null),
> +      docShell: browser.docShell,
> +    });
> +    context.toolbox = toolbox;
> +    context.browser = browser;

I'm not an expert of WebExtensions codebase,
but doesn't that means we are exposing `toolbox` and `browser` to the extension page?

Comment 19

[James Long (:jlongster)]

What's the status of this? If I apply the patches am I able to add a new tool using WebExtensions yet? I'd love to help document this process.

Comment 20

[Blake Winton (:bwinton) (:☕️)]

I applied the patches a while ago, and added a catgifs panel to Whimsy, as mentioned in comment 4…
(I'm not sure what the current status is, though.)

Comment 21

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to James Long (:jlongster) from comment #19)
> What's the status of this? If I apply the patches am I able to add a new
> tool using WebExtensions yet? I'd love to help document this process.

Sorry for lagging on this patch (I was pretty short on time in the last month and I tried to shorten my bugzilla queue as much as possible by removing the higher priority issues first)

Thanks to the first two versions of this patch I've collected a very good amount of feedbacks from both sides (from the webextension and devtools point of views), and I'm going to rewrite it accordingly (e.g. in the previous version of the patch there was a separate actor for the webextension devtools api needs, it was my first choice and Alex prefers it as well)

In the mean time I'm attaching a rebased version of the current patch (in case you want to try it before I have completed and uploaded the new patch version), it applies cleanly and the test case passes (but I've not tried my backbone debugger addon port on it, so I'm not 100% sure that it works completely as before)

Comment 22

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: prototype-open-extension-devtools-API-v2-rebased.patch

This patch is the rebased version of the prototype-open-extension-devtools-API-v2.patch

Comment 23

[Daniele "Mte90" Scasciafratte]

Any news for that ticket?

Comment 24

[Kris Maglione [:kmag]]

Luca, have you had a chance to look into this again recently? Is there anything I can do to help?

Comment 25

[Luca Greco [:rpl] [:luca] [:lgreco]]

In the last weeks I've started to plan and split this issue into smaller steps, here is a Google Doc with a more details description of the current roadmap (and the step that compose it):

- https://docs.google.com/document/d/1nHlef_9wr2mi9e3Ad_RzkDcfG-uY4XGrDt6LhrOXTGs

I'm currently working on the step 1 (fix schema loading incompatibilities related to the devtools API JSON schema files, opened as Bug 1290901) and on the step 2 (import the DevTools schema and create the ext-devtools*.js modules).

Comment 26

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I noticed a post to dev-developer-tools [1] summarizing the current state and primary goals of WebExtensions API for DevTools. Here is some feedback to it - related to some existing API available in Firebug and used by many extensions back then [2]. It could be helpful inspiration for new WebExtensions API design.

The following list represents high-level summary
of Firebug API for extensions:

* Toolbox Panel: an extension should be able to add a new panel (this is already covered in bug 1300587). 

* Side Panel: It should also be possible to add new side-panels (for specific main panels).

* Toolbox Toolbar: it should be possible to add new buttons in the main Toolbox toolbar.

* Context Menu: there should be API allowing appending new context menu items. The API should provide appropriate context so, an extension is able to decide whether to add new item or not.

* Search Box: the search/filter field UI is implemented as shared widget across all panels. This ensures the same search/filter UX for all panels. There should be API allowing extensions to integrate this UI/UX with new panels.

* Options: Firebug implemented options as customizable list of menu items. There is whole panel in case of Fx DevTools. There should be API allowing adding new options.

* Notifications: Extensions should be able to show notification (through NotificationBar component)



Also, Firebug exposed some internal concepts that helped extension developer to design and build extensions:

* Life cycle: extensions should be able to handle various create/destroy events or callback for initialization/cleanup. This can be related to various entities as the Toolbox, target Window (reload) etc.

* Data storage and persistence: where extensions can store data? (browser session data as well as cross session data)

* UI Rendering: how extensions can customize rendering of existing parts. E.g. customize how HTML elements are rendered in the Inspector panel.

* Extension Points - extension points e.g. handling events, injecting hooks, replacing existing components, decorating existing UI, etc.

* Remoting - dealing with remote features.

* Tests, Logging - additional support for development


Honza


[1] https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/98ynBOlkyLs
[2] https://getfirebug.com/wiki/index.php/Firebug_Extensions

Comment 27

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Jan Honza Odvarko [:Honza] from comment #26)
> I noticed a post to dev-developer-tools [1] summarizing the current state
> and primary goals of WebExtensions API for DevTools. Here is some feedback
> to it - related to some existing API available in Firebug and used by many
> extensions back then [2]. It could be helpful inspiration for new
> WebExtensions API design.
> 
> The following list represents high-level summary
> of Firebug API for extensions:
> 
> ...

Hi Honza,
Thanks a lot for the above list of devtools addons features. 

I briefly looked into each of them, and it looks that some of these features could have a correspondent feature in the APIs already provided by the "devtools" extension API namespace, other have correspondent feature in the other APIs (e.g. data storages are provided by the "storage" API namespace) and some are probably completely missing from the APIs that Chrome provides.

I'm going to make a list of notes about this cases (features that exists, the one that exists but with a different API and/or behavior, features that are provided in other existent API namespaces, and features that doesn't seem to have a correspondent) so that we can have a chat about them and then file some new issues for these follow ups.

Comment 28

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Btw. this document might be useful:
Bug 1305979 - Improve API for adding a new side panels

Honza

Comment 29

[Andrei Petcu]

I'm giving a beer to the dev that implements this. 10$ on Bountysource :) I know it's not much but I can't wait for Chrome DevTools extensions to work in Firefox also :) I might increase the amount in the future.

Comment 30

[Luca Greco [:rpl] [:luca] [:lgreco]]

I'm closing this tracker in favor of the new one (Bug 1370525): 
this tracker has reached is purpose (provide the initial subset of the WebExtensions devtools APIs) and keeping it open is now more misleading then useful.

The initial subset of the devtools APIs provided includes:

- the new devtools page and devtools panel contexts
- partial support of the devtools.inspectedWindow API namespace (in particular tabId, eval and reload and the $0 and inspect helpers in the code executed in the inspected window using eval)
- partial support of the devtools.network API namespace (in particular the onTabNavigated API event)
- partial support of the devtools.panels API (in particular panels.create, the panels.themeName property, panels.onThemeChanged event and the panel.onShown and onHidden events)

These APIs have been landed in Firefox 54, with the addition of panels.themeName, panels.onThemeChanged and the $0 and inspect eval helpers, which have been landed in Firefox 55.  

All the remaining dependencies that have not been closed with this tracker are now dependencies of the new tracker (Bug 1370525).