Replace dummy.xhtml with an about:blank content viewer
Categories
(WebExtensions :: General, task, P2)
Tracking
(Not tracked)
People
(Reporter: bgrins, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
This file https://searchfox.org/mozilla-central/source/toolkit/components/extensions/dummy.xul is an empty chrome xul file (now loaded as an HTML document) used for extensions. Looking at the callers, I think this could be just replaced with an empty .html file instead (nothing seems to care that there's a <window> root element): https://searchfox.org/mozilla-central/search?q=dummy.xul&path=
Comment 1•5 years ago
|
||
The reason that we initially had to use a XUL document is that when we used an HTML function, the lack of a layer manager caused us to never trigger layout, which caused our browsers to never get layout frames, which caused them to never get frameloaders.
That likely doesn't matter anymore, given that XULDocument and HTMLDocument are now the same thing, in which case, we should be able to just create an about:blank content viewer and get rid of the dummy document altogether. But it needs testing first.
Updated•5 years ago
|
Reporter | ||
Comment 2•5 years ago
|
||
(In reply to Kris Maglione [:kmag] from comment #1)
The reason that we initially had to use a XUL document is that when we used an HTML function, the lack of a layer manager caused us to never trigger layout, which caused our browsers to never get layout frames, which caused them to never get frameloaders.
Good to know. Even though it's an html document now, it still uses a <xul:window> which could change how layers are working so it will definitely need testing.
That likely doesn't matter anymore, given that XULDocument and HTMLDocument are now the same thing, in which case, we should be able to just create an about:blank content viewer and get rid of the dummy document altogether. But it needs testing first.
But don't we still need a chrome document so that we stuff like creating browsers https://searchfox.org/mozilla-central/rev/e18057a9613ffda06dfd3640209ca234ed7dc37d/toolkit/components/extensions/ExtensionParent.jsm#1397? We support dummy.xul with the browser custom element (https://searchfox.org/mozilla-central/rev/e18057a9613ffda06dfd3640209ca234ed7dc37d/toolkit/content/customElements.js#743-753) and none of that code would run if we don't match the isSystemPrincipal check at https://searchfox.org/mozilla-central/rev/e18057a9613ffda06dfd3640209ca234ed7dc37d/toolkit/components/processsingleton/CustomElementsListener.jsm#16
Comment 3•5 years ago
|
||
We do still need a chrome document, but we already call createAboutBlankContentViewer with a chrome principal, for a complicated set of reasons, so we already have one.
Comment 4•5 years ago
|
||
That said... the check in question also excludes about:blank, which would obviously need to be fixed.
Reporter | ||
Comment 5•5 years ago
|
||
(In reply to Kris Maglione [:kmag] from comment #4)
That said... the check in question also excludes about:blank, which would obviously need to be fixed.
I guess we can't differentiate between this about:blank and any others? bz made a change to intentionally exclude about:blank in Bug 1528762, so I'd be worried about removing that check.
Comment 6•5 years ago
|
||
Not really. I suppose we could manually inject them into that window, though. Maybe by having the custom elements code inject a function we can call to do the things it normally does.
Reporter | ||
Comment 7•5 years ago
|
||
(In reply to Kris Maglione [:kmag] from comment #6)
Not really. I suppose we could manually inject them into that window, though. Maybe by having the custom elements code inject a function we can call to do the things it normally does.
Is there a particular benefit to using the about:blank content viewer instead of a chrome HTML document? In general I'd prefer to not add new entry points or special cases for how our chrome CEs get loaded in various documents in order to keep things more consistent (i.e. we can always assume we aren't going to be running any of this stuff in a document that doesn't match https://searchfox.org/mozilla-central/rev/e18057a9613ffda06dfd3640209ca234ed7dc37d/toolkit/components/processsingleton/CustomElementsListener.jsm#16-23).
Comment 8•5 years ago
|
||
Yes. Having to load a document from a channel is considerably more expensive. about:blank content viewers are created synchronously using essentially pure DOM operations.
Comment 9•5 years ago
|
||
Reporter | ||
Comment 10•5 years ago
|
||
(In reply to Kris Maglione [:kmag] from comment #8)
Yes. Having to load a document from a channel is considerably more expensive. about:blank content viewers are created synchronously using essentially pure DOM operations.
OK, probably worth trying it with that as well. We can at least test with https://phabricator.services.mozilla.com/D46629 to make sure we aren't tripping any of these layer issues you mentioned.
Comment 11•5 years ago
|
||
Should we be loading the URI like this https://hg.mozilla.org/try/rev/8eaa3abe782ef960f331f66a57d940ffea02f881#l2.16?
The current patch that I have up has a few unexpected failures (https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=267760663&revision=5d3c62411eb00c449758cbe77a5fedbb0903e008) when we change to an about:blank content viewer.
Updated•5 years ago
|
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Comment 13•4 years ago
|
||
(In reply to Emma Malysz from comment #11)
Should we be loading the URI like this https://hg.mozilla.org/try/rev/8eaa3abe782ef960f331f66a57d940ffea02f881#l2.16?
The current patch that I have up has a few unexpected failures (https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=267760663&revision=5d3c62411eb00c449758cbe77a5fedbb0903e008) when we change to an about:blank content viewer.
I would expect that to work, but the artifacts for that try push are unfortunately gone.
Updated•3 years ago
|
Updated•3 years ago
|
Updated•2 years ago
|
Description
•