739078 - Extension that calls loadAndRegisterSheet doesn't work correctly in e10s Fennec

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: Extension Compatibility, defect)

Description

[Bill Gianopoulos [:WG9s]]

User style sheets registered by an extension via bootstrap.js seem to have no effect on Android XUL builds.

The reason I have given this a "major" Severity is that in order to resolve bug 295193, an extension has been developed to package and include the math fonts required for proper display of MathML.  This extension does not work on Android XUL builds because of this issue.

It is especially important for this to work on mobile devices which typically do not permit users installing additional fonts without "rooting" the device.

Comment 1

[Bill Gianopoulos [:WG9s]]

Attachment: Simple extension illustrating the issue

This extension illustrates the issue.  It merely adds a stylesheet that sets the body background color to pink.  It works under Linux, Windows and Android native.

It does not work on current Mozilla-central Android XUL builds.

Comment 2

[Matt Brubeck (:mbrubeck)]

I don't think this is a general problem with bootstrapped add-ons.  Other bootstrapped add-ons like Phony, Show Tooltips, Full Screen, etc., work fine in XUL Fennec.

More likely this extension needs to be modified to support Electrolysis, which is enabled by default in XUL Fennec.  It looks like it currently loads the stylesheet in the chrome process, while it probably needs to load it in the content process.  For details, see https://wiki.mozilla.org/Mobile/Fennec/Extensions/Electrolysis

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Another note. XUL Fennec used to use loadAndRegisterSheet but when we moved to e10s (and component manifests) we switched to using "agent-style-sheets" component categories:
http://mxr.mozilla.org/mozilla-central/source/mobile/xul/components/MobileComponents.manifest#40

Comment 4

[[e-mail]]

I didn't think extension manifests got loaded into content processes... but even if they do, how would this work for a restartless extension?

Comment 5

[Bill Gianopoulos [:WG9s]]

I managed to get this simple extension to work this way, but as said above, I can't figure out how to do this in a restartless extension.

Comment 6

[Bill Gianopoulos [:WG9s]]

Oh looks like I should have read Neil's comment more closely.  The stylesheet added by that method seems to only work on pages like about:home.  and not on content pages.

Comment 7

[Matt Brubeck (:mbrubeck)]

If you want to try using loadAndRegisterSheet in the content process, you can create a separate script (let's call it "content.js") that will run in the content process, and call messageManager.loadFrameScript("content.js", true) from bootstrap.js to load that file in the content process.

To shut down the extension on uninstall/disable, the parent process can send a message to the frame script using messageManager.sendAsyncMessage, and the content script can use addMessageListener to listen for that message.

You can see https://hg.mozilla.org/users/mbrubeck_mozilla.com/biggertext/ for some example code of a small add-on that uses a content script (though this is not a restartless add-on, sorry).

Comment 8

[Bill Gianopoulos [:WG9s]]

(In reply to Matt Brubeck (:mbrubeck) from comment #7)
> If you want to try using loadAndRegisterSheet in the content process, you
> can create a separate script (let's call it "content.js") that will run in
> the content process, and call messageManager.loadFrameScript("content.js",
> true) from bootstrap.js to load that file in the content process.

OK.  Either I am doing something stupid or there is a bug here.
hen I try to do the messageMager.loadFrameScript from bootstrap.js, I get an Exception saying messageManager is not defined.  However, looking through other bugs, it would appear it is supposed to be defined globally.

Comment 9

[Bill Gianopoulos [:WG9s]]

Oh. I should have mentioned I am doing this from within the startup function.  I would have presumed that was the correct place to do this.

Comment 10

[Wesley Johnston (:wesj)]

bootstrap.js runs in its own magic context as well. If you're doing something like:

http://starkravingfinkle.org/blog/2011/01/bootstrap-jones-adventures-in-restartless-add-ons/

you should be able to the loadIntoWindow function to something like:

function loadIntoWindow(window) {
  window.messageManager.loadFrameScript();
}

Comment 11

[Bill Gianopoulos [:WG9s]]

OK. I also found this. http://starkravingfinkle.org/blog/2011/01/bootstrap-jones-adventures-in-restartless-add-ons/  I will try to work on this tomorrow.

Thanks to everyone who has helped.

Comment 12

[Bill Gianopoulos [:WG9s]]

Attachment: Extension re-written for e10s

OK.  Based on all of the information posted above I managed to get this all to be a restartless extension on Desktop, Native Android and Android XUL.

The problem is that under Android XUL, the stylesheet is STILL only being applied to chrome pages.

Comment 13

[Bill Gianopoulos [:WG9s]]

Attachment: Extension re-written for e10s - update

Minor changes.

1.  Remove contentaccesible flag in chrome. manifest.  I had added this in an attempt to get it to work on non-chrome pages in Android XUL, but it made no difference.

2.  I am no longer setting the "delayed load flag" as I don;t think this should be necessary because of the way it is now waiting for the onOpenWIndow event.  This avoids having to worry about cleaning these up later.

3.  The big change is it fixes an issue that occurred if you removed the extension without first disabling it.  Seems you need to have the uninstall function call the shutdown function in this case.

I posted this just in case someone later on tries to use this as a starting point for their extension.

Comment 14

[Bill Gianopoulos [:WG9s]]

So, is the fact that this style sheet does not apply to chrome windows an e10s issue, or a mobile XUL build issue?

Comment 15

[Matt Brubeck (:mbrubeck)]

(In reply to Bill Gianopoulos [:WG9s] from comment #14)
> So, is the fact that this style sheet does not apply to chrome windows an
> e10s issue, or a mobile XUL build issue?

I'm not sure off-hand... it might be that you would also need to call loadAndRegisterSheet in the parent process (e.g. from bootstrap.js) for it to apply to the top-level chrome window.  If that doesn't work either, it might be a XUL Fennec bug.

Comment 16

[Matt Brubeck (:mbrubeck)]

This bug seems to be resolved.

Comment 17

[Bill Gianopoulos [:WG9s]]

Actually the real issue is NOT that the stylesheet is only applied to chrome pages and not to content, because there are chrome pages on which it is NOT applied.  The difference seems to be it is only applied on XUL pages.

So, anyway the original issue I had is that there seems to be no way for an extension to add a stylesheet to be added to content on Android XUL still exists.  Should I just file a new bug using this same extension as the testcase?

Comment 18

[Matt Brubeck (:mbrubeck)]

We can reopen and handle it here.

Comment 19

[Bill Gianopoulos [:WG9s]]

(In reply to Matt Brubeck (:mbrubeck) from comment #15)

> I'm not sure off-hand... it might be that you would also need to call
> loadAndRegisterSheet in the parent process (e.g. from bootstrap.js) for it
> to apply to the top-level chrome window.  If that doesn't work either, it
> might be a XUL Fennec bug.

That did not appear to make make any difference.

I have a couple of other ideas of things to try to further define the issue, but they both involve questions.

1.  Is there anywhere on Android where a userContent.css file could be placed where it would be recognized?  If not I might try a hacky patch to have it look someplace and see what happens if I load a stylesheet that way.

2.  Is there some way to run Android XUL without e10s?  Either via a config setting or some kind of patch in the build?  It does not have to be fast, just be functional.  I just want to see if doing that results in the extension working as expected or if it still fails.

Comment 20

[Frédéric Wang (:fredw)]

Attachment: non-restarless extension using the category "user-style-sheets"

Comment 21

[Frédéric Wang (:fredw)]

(In reply to Frédéric Wang from comment #20)
> Created attachment 611026 [details]
> non-restarless extension using the category "user-style-sheets"

FYI, this version does not seem to work either with Android XUL build...

Comment 22

[Matt Brubeck (:mbrubeck)]

(In reply to Bill Gianopoulos [:WG9s] from comment #19)
> 1.  Is there anywhere on Android where a userContent.css file could be
> placed where it would be recognized?  If not I might try a hacky patch to
> have it look someplace and see what happens if I load a stylesheet that way.

It looks like userContent.css and other ways of adding user styles are all equally broken in e10s Fennec; see bug 609602.

> 2.  Is there some way to run Android XUL without e10s?  Either via a config
> setting or some kind of patch in the build?  It does not have to be fast,
> just be functional.  I just want to see if doing that results in the
> extension working as expected or if it still fails.

You can set the pref "browser.tabs.remote" to false, to disable e10s for testing purposes.  (The pref will only affect new tabs; you'll need to close and re-open any existing tabs.)  Note that some things are broken in non-remote tabs, including zooming.

Comment 23

[Chris Peterson [:cpeterson]]

We don't need to track these XUL Fennec bugs for desktop e10s.

Comment 24

[BMO Automation]

Closing all opened bug in a graveyard component