Closed Bug 1579066 Opened 7 years ago Closed 6 years ago

Make the "browser" object accessible to the content scripts of the legacy XUL extensions with overlays, to be able to communicate with the WebExtensions background scripts

Categories

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Product:
Thunderbird
Component:
Add-Ons: Extensions API
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INVALID

People

(Reporter: mihaicodrean, Unassigned)

Details

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

I'm using a legacy XUL extension with a sidebar overlay and I'd like to communicate with the WebExtensions background script. However, there is no "browser" object accessible from the content script, to be able to do a browser.runtime.sendMessage(), as detailed here: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Content_scripts#Communicating_with_background_scripts

Alternatively, support for the sidebar_action in manifest.json could be added, to then be able to do runtime.getBackgroundPage(), as detailed here: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/user_interface/Sidebars

Magnus, we should start managing bugs and enhancement requests for WE API.

This would be a big enhancement. I've been able to convert my overlay extensions to eliminate chrome.manifest (yes, that means locale, overlay dom nodes and script, skin - all of it) and remove legacy:xul. It was made harder without access to storage and i18n directly in chrome code.

There's some reason Fx didn't do it, why not use the apis in chrome code? But since they have no chrome code extensions, like we must, it's a bigger need in Tb. Plus, I was tired of the overlay console pollution..

Jorg, is this complicated to address?

Flags: needinfo?(jorgk)

Sorry, need to forward this to the expert.

Flags: needinfo?(jorgk) → needinfo?(geoff)

Got it, thanks Jorg.

In short: yes, this is complicated to address. Especially since we don't have embedded WebExtensions any more, and in fact have legacy extensions embedded in WebExtensions.

But first, let's establish exactly what is needed here. Is it some way, any way, to communicate with the WE runtime? That's potentially much easier than trying to provide all the APIs to the legacy bits.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(geoff)

(In reply to Geoff Lankow (:darktrojan) from comment #6)

Is it some way, any way, to communicate with the WE runtime?

Yes, any way that I could communicate from my legacy sidebar XUL extension to its background WE script would be great. The background script will do the heavy lifting (as a controller in MVC / view-model in MVVM), while the sidebar will be its "view", displaying live results & have buttons to trigger actions in the background script.

What do you suggest?

PS: Please let me know if you don't want me to add the needinfo flag to get your attention.

Flags: needinfo?(geoff)

Not sure what's difficult here.

  1. In WE, there would be a runtime.sendNotification() which wraps nsIObserverService.notifyObservers(); the chrome code would be responsible for adding an observer on load or init, like usual; it would be in the form Services.addNotification(). I've used an experiments.js middleman for this functionality, which has access to both Services.* and browser.*. Because background.js is loaded way before overlay code, the middleman has an observer waiting for overlay code to notify it is ready before the middleman sends its notification with WE context data.

  2. In chrome, there would be a Services.sendNotification() which wraps notifyObservers() and mimics runtime.sendMessage(); the WE context code would add runtime.onNotification.addListener(). An onNotification namespace would be used to distinguish from onMessage (inter WE messaging).

  3. Both sendNotification() and addNotification(), plus OnNotification, ensure the right |topic| token is used internally, constructed to include the extension id, etc.

I follow what you're thinking alta88, would you mind sharing what you've done already? That probably makes more sense than starting from scratch.

Flags: needinfo?(geoff)

You can look at AttachmentCount or MoreLayouts for Tb68. Besides the background.js and experiments.js and schema.json, you need to look at the main chrome script's onLoad() for its registration and the Observer for handling a response, and the very bottom where readyState must be tested to differentiate startup vs. enabled in addon manager.

I didn't have a need to send anything to background.js but I'm sure you can figure out how the reverse would be done. It would be a lot easier just to have |browser| access for chrome code, but they went out of their way not to, and also especially obfuscated storage.local (not available in an experiment afaict).

https://bitbucket.org/alta8888/attachmentcount/wiki/Home
https://bitbucket.org/alta8888/morelayouts/wiki/Home

OT - Something like AttachmentCount shows missing api stuff:

  1. In mailTabs, things like sortType, sortOrder are returned but not secondarySortType or secondarySortOrder from the db view. Maybe others should be also.
  2. I haven't seen any doc for custom columns support in a WE. Which is sad since I spent some time making them not suck years ago.. Also need to return curCustomColumn and secondaryCustomColumn from db view.
  3. For the getFull() api, MsgHdrToMimeMessage() is used. But that doesn't work with newsgroups and it really should.

I think the general thought process here is wrong.

The WebExtension part - the new bit - should be thought of as the controller, and the sidebar legacy the agent. You can use a WebExtension experiment to create a listener type API. When your add-on starts up, it registers the listener through the API, and then your sidebar can "callback" to the listener via that API. It isn't perfect, but it should work.

As you've already said, implementing the sidebar_action API is actually the better option - since that would allow your WebExtension to run the sidebar, the sidebar would be loaded in the WebExtension scope, and you then wouldn't need the legacy add-on. Maybe that should be treated as the actual request here?

I think the general thought process here is wrong. The WebExtension part - the new bit - should be thought of as the controller, and the sidebar legacy the agent.

That's what I was saying as well, in comment 7.

You can use a WebExtension experiment to create a listener type API. When your add-on starts up, it registers the listener through the API, and then your sidebar can "callback" to the listener via that API. It isn't perfect, but it should work.

Is there an example somewhere of how to do this?

As you've already said, implementing the sidebar_action API is actually the better option - since that would allow your WebExtension to run the sidebar, the sidebar would be loaded in the WebExtension scope, and you then wouldn't need the legacy add-on. Maybe that should be treated as the actual request here?

Whichever makes more sense, yes.

Flags: needinfo?(standard8)

(In reply to Mihai from comment #12)

You can use a WebExtension experiment to create a listener type API. When your add-on starts up, it registers the listener through the API, and then your sidebar can "callback" to the listener via that API. It isn't perfect, but it should work.

Is there an example somewhere of how to do this?

The example extension does this with browser.myapi.onToolbarClick.

Flags: needinfo?(standard8)

Thanks Mark, I'll check it out.

Mark> [...] and then your sidebar can "callback" to the listener via that API.

How can it, since there's no "browser" object there?

Flags: needinfo?(standard8)

(In reply to Mihai from comment #15)

Mark> [...] and then your sidebar can "callback" to the listener via that API.

How can it, since there's no "browser" object there?

Your experiment API will need to set a listener on your code in the sidebar, a bit like what is happening here: https://github.com/thundernest/sample-extensions/blob/master/experiment/implementation.js#L23-L39

So the background.js (or whatever you call it), registers a listener via the experiment API, then the experiment API registers a listener on your sidebar.

Flags: needinfo?(standard8)

(In reply to Mark Banner (:standard8) from comment #16)

Thanks Mark, I got the Experiment API from the sample working in my extension, for a test.

However, the next roadblock is that it appears that my custom objects passed through the Experiment API using the schema type "any" get cloned, as opposed to pass-by-reference as I would have expected. My aim is to be able to manipulate the exact same object from the background script, while the windowListener will monitor for changes, to update the UI. There is also an "in-memory-datasource" (nsIRDFDataSource) to be live-displayed in a XUL tree, so quite large.

Did I miss anything when it comes to the schema type "any"?

Flags: needinfo?(standard8)

I think they always get cloned. (Maybe someone can confirm.)

Note though that RDF is now completely removed, so any nsIRDFDataSource you would have been using isn't available anymore.

(In reply to Magnus Melin [:mkmelin] from comment #18)

I think they always get cloned. (Maybe someone can confirm.)

Is that a design constraint or there could be a schema type that could do it differently?

Note though that RDF is now completely removed, so any nsIRDFDataSource you would have been using isn't available anymore.

Are there any alternatives for observable collections (similar to nsIRDFDataSource) and controls (similar to the XUL tree) to bind them to?

AFAIK, it's designed that way and not really changeable.

You'd have to look at what they were replaced with in core. I don't recall what used to be used for what, since most of it has been gone since years. Feeds was one of the last ones to be converted over, you can check https://searchfox.org/comm-central/rev/3979888eb7841cb8774d099c087c12d0a44c1965/mailnews/extensions/newsblog/content/FeedUtils.jsm#1488

Understood & thanks for the tips, I'll dig.

The cloning would be designed that way, as this would provide separation between the extension objects, and the core objects. So one couldn't inadvertently change the other's objects and cause side effects that weren't expected.

Flags: needinfo?(standard8)

(In reply to Mark Banner (:standard8) from comment #22)

The cloning would be designed that way, as this would provide separation between the extension objects, and the core objects. So one couldn't inadvertently change the other's objects and cause side effects that weren't expected.

Got it. The downside is that it's not possible to apply a true MVC/MVP/MVVM pattern, where the background script would be the controller / view-model etc.

(In reply to Magnus Melin [:mkmelin] from comment #20)

AFAIK, it's designed that way and not really changeable.

You'd have to look at what they were replaced with in core. I don't recall what used to be used for what, since most of it has been gone since years. Feeds was one of the last ones to be converted over, you can check https://searchfox.org/comm-central/rev/3979888eb7841cb8774d099c087c12d0a44c1965/mailnews/extensions/newsblog/content/FeedUtils.jsm#1488

As far as I understand, nsITreeView implementations are now set as the XUL tree.view property - sounds good. However, no matter what I set the tree.view to in my extension, checking the property right after returns "null". Is this a permission issue? I see that XULTreeElement::SetView method checks the caller type to be System.

Flags: needinfo?(mkmelin+mozilla)

Likely yes? But it all depends on what you're doing where.

Flags: needinfo?(mkmelin+mozilla)

(In reply to Magnus Melin [:mkmelin] from comment #25)

Likely yes? But it all depends on what you're doing where.

What do you mean? Yes for being a permission issue? What I'm doing is having a XUL "tree" control in a sidebar in my extension, that I would like to dynamically fill with data (coming from the WebExtensions background script, via an Experiment API).

I mean I don't have your code so it's hard to say anything. You could always compile thunderbird and add a check there to see if that's the issue.

(In reply to Magnus Melin [:mkmelin] from comment #27)

[...] You could always compile thunderbird and add a check there to see if that's the issue.

Thanks, did that, only to find out the hard way that the XUL tree needs an empty <treechildren/> element when operating in "Custom tree view" mode, as opposed to using RDFs - from which I have migrated from.

This bug has gone cold, and it's drifted so far from the original problem that I don't think it is worthwhile leaving open.

Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.