Outline of a plan to remove all XUL documents from mozilla-central

246 views
Skip to first unread message

Brian Grinstead

unread,
May 14, 2019, 11:24:44 AM5/14/19
to Mozilla dev-platform mailing list mailing list
To prepare for browser.xhtml (bug 1533881 <https://bugzilla.mozilla.org/show_bug.cgi?id=1533881>), we’ve been smoothing over differences between XUL documents and chrome HTML documents (bug 1453783 <https://bugzilla.mozilla.org/show_bug.cgi?id=1453783>). We are now working out a plan for removing XUL documents entirely from mozilla-central (bug 1540278 <https://bugzilla.mozilla.org/show_bug.cgi?id=1540278>). There are currently around 1530 xul documents in the tree <https://github.com/bgrins/xul-analysis/blob/master/file-lists/2019-05-09.txt> and around 1400 of these are in test directories.

We’re early in the process right now but wanted to circulate the plan to let people know about upcoming changes and give a chance to provide feedback. If there aren’t objections, we think step 1 could be completed shortly after the merge (2019-05-20) and that we could start work on the following steps in short order. The outline of the plan is:

Load all XUL documents as XHTML using the prototype cache. This doesn’t require any file renaming, we will just detect a .xul file and act like it’s .xhtml. This is tracked in bug 1550801 <https://bugzilla.mozilla.org/show_bug.cgi?id=1550801>. There are only a few known changes required to make this work properly, for example:
Fixup calls to `document.createElement` to `document.createXULElement` since the default namespace will now be HTML (Bug 1551320 <https://bugzilla.mozilla.org/show_bug.cgi?id=1551320>).
Remove any remaining references to XULDocument. Most of these have been removed already, but there are some callers that didn’t apply to the main browser document that haven’t been removed yet (Bug 1551344 <https://bugzilla.mozilla.org/show_bug.cgi?id=1551344>)
This does _not_ require changing whitespace aware API callers (i.e. calls to .childNodes->.children), since the prototype cache drops whitespace in both XUL and XHMTL.
Delete the XULDocument implementation.
For files where there are no (important) XUL elements in the markup, rename .xul->.html. This requires doing at least:
Fixup whitespace-aware API callers (i.e. calls to .childNodes->.children).
Write a tool to parse and automatically convert these documents as much as possible. For instance, stop using self closing tags, migrate `<xul:window>`->`<html>`, convert ProcessingInstruction stylesheets to `<link>`, etc.
Update bugs and comments referencing the old file name
For files that don’t match the heuristics from (3), rename .xul->.xhtml. This is simpler than (3), since we don’t have to worry about whitespace aware API, parsing issues, etc. In this case everything should work identically with the file name changed. The main extra task here is:
Update bugs and comments referencing the old file name
For the now-xhtml documents from (4), apply a subset of changes from (3). For instance, migrate `<xul:window>`->`<html>`, convert ProcessingInstruction stylesheets to `<link>`, etc).

A variation on this plan would be to move step 3 until after step 4 - essentially do a blanket rename of all .xul files to .xhtml shortly after step 1 stuck. This would make it faster to complete the removal of xul files from the tree and get out of the intermediate state where we have .xul files loaded as HTML documents. However, this would come at the cost of more filename churn - tests that are ultimately destined for .html would be renamed .xul->.xhtml->.html instead of only .xul->.html. Renaming files has a cost because it can break in-flight patches, make it harder to search history, and requires updating bugs referencing the old file name (for example, for intermittent tracking bugs <https://bugzilla.mozilla.org/show_bug.cgi?id=1543834>). Because of that, we are recommending the outline as-is instead of the variation above.

Thanks,
Brian

Brian Grinstead

unread,
May 14, 2019, 11:32:25 AM5/14/19
to Mozilla dev-platform mailing list mailing list
It looks like the post lost formatting on the way - most importantly the
numbers next to each step and nesting in the list. I made a formatted
version of this post at
https://docs.google.com/document/d/1rEPcu7ei-kK5Dvt7sBgqa_vP37QFIUtVd33JkCK1mvM/edit
.

Here's a (hopefully) fixed version of the list:

1. Load all XUL documents as XHTML using the prototype cache. This doesn’t
require any file renaming, we will just detect a .xul file and act like
it’s .xhtml. This is tracked in bug 1550801. There are only a few known
changes required to make this work properly, for example:
a) Fixup calls to `document.createElement` to
`document.createXULElement` since the default namespace will now be HTML
(Bug 1551320).
b) Remove any remaining references to XULDocument. Most of these have
been removed already, but there are some callers that didn’t apply to the
main browser document that haven’t been removed yet (Bug 1551344)
c) This does _not_ require changing whitespace aware API callers (i.e.
calls to .childNodes->.children), since the prototype cache drops
whitespace in both XUL and XHMTL.
2. Delete the XULDocument implementation.
3. For files where there are no (important) XUL elements in the markup,
rename .xul->.html. This requires doing at least:
a) Fixup whitespace-aware API callers (i.e. calls to
.childNodes->.children).
b) Write a tool to parse and automatically convert these documents as
much as possible. For instance, stop using self closing tags, migrate
`<xul:window>`->`<html>`, convert ProcessingInstruction stylesheets to
`<link>`, etc.
c) Update bugs and comments referencing the old file name
4. For files that don’t match the heuristics from (3), rename .xul->.xhtml.
This is simpler than (3), since we don’t have to worry about whitespace
aware API, parsing issues, etc. In this case everything should work
identically with the file name changed. The main extra task here is:
d) Update bugs and comments referencing the old file name
5. For the now-xhtml documents from (4), apply a subset of changes from
(3). For instance, migrate `<xul:window>`->`<html>`, convert
ProcessingInstruction stylesheets to `<link>`, etc).

On Tue, May 14, 2019 at 8:24 AM Brian Grinstead <bgrin...@mozilla.com>
wrote:

> To prepare for browser.xhtml (bug 1533881
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1533881>), we’ve been
> smoothing over differences between XUL documents and chrome HTML documents (bug
> 1453783 <https://bugzilla.mozilla.org/show_bug.cgi?id=1453783>). We are
> now working out a plan for removing XUL documents entirely from
> mozilla-central (bug 1540278
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1540278>). There are
> currently around 1530 xul documents in the tree
> <https://github.com/bgrins/xul-analysis/blob/master/file-lists/2019-05-09.txt>
> and around 1400 of these are in test directories.
>
> We’re early in the process right now but wanted to circulate the plan to
> let people know about upcoming changes and give a chance to provide
> feedback. If there aren’t objections, we think step 1 could be completed
> shortly after the merge (2019-05-20) and that we could start work on the
> following steps in short order. The outline of the plan is:
>
>
> 1. Load all XUL documents as XHTML using the prototype cache. This
> doesn’t require any file renaming, we will just detect a .xul file and act
> like it’s .xhtml. This is tracked in bug 1550801
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1550801>. There are only
> a few known changes required to make this work properly, for example:
> 1. Fixup calls to `document.createElement` to
> `document.createXULElement` since the default namespace will now be HTML (Bug
> 1551320 <https://bugzilla.mozilla.org/show_bug.cgi?id=1551320>).
> 2. Remove any remaining references to XULDocument. Most of these
> have been removed already, but there are some callers that didn’t apply to
> the main browser document that haven’t been removed yet (Bug 1551344
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1551344>)
> 3. This does _not_ require changing whitespace aware API callers
> (i.e. calls to .childNodes->.children), since the prototype cache drops
> whitespace in both XUL and XHMTL.
> 2. Delete the XULDocument implementation.
> 3. For files where there are no (important) XUL elements in the
> markup, rename .xul->.html. This requires doing at least:
> 1. Fixup whitespace-aware API callers (i.e. calls to
> .childNodes->.children).
> 2. Write a tool to parse and automatically convert these documents
> as much as possible. For instance, stop using self closing tags, migrate
> `<xul:window>`->`<html>`, convert ProcessingInstruction stylesheets to
> `<link>`, etc.
> 3. Update bugs and comments referencing the old file name
> 4. For files that don’t match the heuristics from (3), rename
> .xul->.xhtml. This is simpler than (3), since we don’t have to worry about
> whitespace aware API, parsing issues, etc. In this case everything should
> work identically with the file name changed. The main extra task here is:
> 1. Update bugs and comments referencing the old file name
> 5. For the now-xhtml documents from (4), apply a subset of changes

Boris Zbarsky

unread,
May 14, 2019, 3:37:55 PM5/14/19
to
On 5/14/19 11:32 AM, Brian Grinstead wrote:
>> 3. For files where there are no (important) XUL elements in the
>> markup, rename .xul->.html.

Brian,

Could you expand on why this is preferable (when possible) to renaming
them to .xhtml? Are there benefits to .html over .xhtml for our
purposes here? Is this mostly around how we deal with our tests, as
opposed to actual parts of the UI?

In general, the plan sounds great!

-Boris

Brian Grinstead

unread,
May 14, 2019, 4:48:30 PM5/14/19
to Boris Zbarsky, dev-pl...@lists.mozilla.org
There isn't any particular reason functionally to go to one vs the other but I think we still generally prefer to get to plain .html if possible. The reasoning is that it's more common and understood by engineers and tooling. It also doesn't have XML-specific additions like CDATA in script tags.

That said, after talking through this again with Brendan we realized it may not end up being worth the effort for existing test files. If we were able to apply some of the incremental changes from step 5 to test .xhtml files that may be good enough. If the cost was low to include step 3 (like, if we had a tool that mostly automated away the process) then I’d prefer to do it. But it could be pragmatic to skip step 3 (at least for test documents). We’ll be working on some tooling anyway for step 5, so will spend a bit of time seeing how hard it looks to be to automate the html doc conversion.

Brian
> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

Dave Townsend

unread,
May 14, 2019, 5:44:19 PM5/14/19
to Brian Grinstead, Boris Zbarsky, dev-platform
Which test files are we talking about here? If they are testing UI widgets,
and our long-term goal is to use html and not xhtml for the UI then those
tests should, at some point, be in html.

On Tue, May 14, 2019 at 1:48 PM Brian Grinstead <bgrin...@mozilla.com>
wrote:

> There isn't any particular reason functionally to go to one vs the other
> but I think we still generally prefer to get to plain .html if possible.
> The reasoning is that it's more common and understood by engineers and
> tooling. It also doesn't have XML-specific additions like CDATA in script
> tags.
>
> That said, after talking through this again with Brendan we realized it
> may not end up being worth the effort for existing test files. If we were
> able to apply some of the incremental changes from step 5 to test .xhtml
> files that may be good enough. If the cost was low to include step 3 (like,
> if we had a tool that mostly automated away the process) then I’d prefer to
> do it. But it could be pragmatic to skip step 3 (at least for test
> documents). We’ll be working on some tooling anyway for step 5, so will
> spend a bit of time seeing how hard it looks to be to automate the html doc
> conversion.
>
> Brian
>
> > On May 14, 2019, at 12:37 PM, Boris Zbarsky <bzba...@mit.edu> wrote:
> >

Gijs Kruitbosch

unread,
May 15, 2019, 9:29:43 AM5/15/19
to
On 14/05/2019 16:32, Brian Grinstead wrote:
>> 1. Load all XUL documents as XHTML using the prototype cache. This
>> doesn’t require any file renaming, we will just detect a .xul file and act
>> like it’s .xhtml. This is tracked in bug 1550801
>> <https://bugzilla.mozilla.org/show_bug.cgi?id=1550801>.

There doesn't seem to be a bug on file listed in that tracking bug for
actually doing the "detect a .xul file and act like it's .xhtml" bit. Is
there one already on file?

I think this step makes me the most nervous - we need to make sure that
we don't change web-observable behaviour, and that the "no remote XUL"
prohibitions continue to apply. In theory, that should all be fine
because of XHTML and namespaces, but I figured it was worth flagging up.

~ Gijs

Brendan Dahl

unread,
May 15, 2019, 11:42:34 AM5/15/19
to Gijs Kruitbosch, dev-platform
https://bugzilla.mozilla.org/show_bug.cgi?id=1550801


On Wed, May 15, 2019 at 6:30 AM Gijs Kruitbosch <gijskru...@gmail.com>
wrote:

Gijs Kruitbosch

unread,
May 15, 2019, 11:45:18 AM5/15/19
to
This was linked already - but comment #0 and the deps of that bug only
mention no longer relying on the XULDocument interface. That's different
from "detect and parse .xul files as XHTML" (at least, I think it is).
I'd expect a separate dependency for that, but I don't see any.

~ Gijs

Brendan Dahl

unread,
May 15, 2019, 11:52:40 AM5/15/19
to Gijs Kruitbosch, dev-platform
Sorry, hit send too early. I'll be doing that work in that bug once the
dependent bugs are finished. The current patch to load XUL as XHTML is
https://hg.mozilla.org/try/rev/95fc313303f452f6052093bbbebc1cf7beb2d9f1 and
overall is getting pretty close to a green try run.

It really shouldn't be that big of a change since most of XULDocument has
been gutted and PrototypeCache support was added to chrome privileged
XHTML.

I'l file a bug to double check we're doing the right thing for remote XUL
still.

On Wed, May 15, 2019 at 8:42 AM Brendan Dahl <bd...@mozilla.com> wrote:

> https://bugzilla.mozilla.org/show_bug.cgi?id=1550801
>
>
> On Wed, May 15, 2019 at 6:30 AM Gijs Kruitbosch <gijskru...@gmail.com>
> wrote:
>

Brian Grinstead

unread,
May 15, 2019, 12:56:24 PM5/15/19
to Dave Townsend, Boris Zbarsky, dev-platform


> On May 14, 2019, at 2:43 PM, Dave Townsend <dtow...@mozilla.com> wrote:
>
> Which test files are we talking about here? If they are testing UI widgets, and our long-term goal is to use html and not xhtml for the UI then those tests should, at some point, be in html.

It's worth breaking the tests down into groups to consider separately. After going through this in more detail and based on discussion coming out of this post, it makes me think the variation where we do a blanket rename of all .xul files to .xhtml might be a better option - it'd certainly be less work at least.

Here are some of the groups of tests I see:

"Reftests" (Number: 364)

These are probably mostly safe to migrate, but there could be different CSS applying on elements (for instance if the root node is <html:html> and not <xul:window>). For that reason I think we could go straight to .xhtml (step 4) and then selectively apply the step 5 cleanups as long as they don't break tests.

"Crashtests" (Number: 147)

I think I'd go straight to .xhtml (step 4), and probably not even do any step 5 cleanups. The reasoning is we don't want to go through each test one by one and decide if we are losing test coverage by making those changes.

“Mochitests that are specifically testing XML features” (Number: ???)

These would need to go straight to .xhtml (step 4) and then selectively apply the step 5 cleanups as long as they don't break tests.

"Mochitests that test built-in widgets" (Number: ~160)

I'm using tests in the `toolkit/content/tests/chrome` folder as a proxy for this. As you mention, we might want these to live in the same document the browser does (which will be xhtml, at least for now). Though in general something a widget that works in .xhtml should also work in .html - the breakage would mostly go the other way (i.e. if a widget relied on the prototype cache parser ignoring whitespace could work in xhtml but not in html). So I think you could argue either way - going straight to .xhtml would maintain the status quo.

"Mochitests that don't care about the test document" (Number: ???)

One example that I had envisioned going straight to .html are the mochitest-chrome tests in xpfe: https://bugzilla.mozilla.org/show_bug.cgi?id=1548152. I picked this folder as a starting point because there are only two tests there (https://searchfox.org/mozilla-central/source/xpfe/appshell/test/test_hiddenPrivateWindow.xul and https://searchfox.org/mozilla-central/source/xpfe/appshell/test/test_windowlessBrowser.xul), and as far as I can tell there's nothing in them that cares whether the test document itself is xul, xhtml, or html.

This is primarily the type of test I had in mind for (3). My suspicion is that there are a lot of tests of that type across the tree. The trick will be finding them - Brendan has a script to find the most common DOM patterns across all the tests, and there are 32 other tests that match that exact DOM structure: https://gist.github.com/bgrins/22a448034748340042bdbf499e219a71#file-25-top-repeated-structures-txt-L426. It's not a guarantee that all of them would be safe to migrate, but spot-checking other tests there it seems to be the case. There are a bunch of other shared DOM structures that seem to be in a similar boat.

Brian

> On Tue, May 14, 2019 at 1:48 PM Brian Grinstead <bgrin...@mozilla.com> wrote:
> There isn't any particular reason functionally to go to one vs the other but I think we still generally prefer to get to plain .html if possible. The reasoning is that it's more common and understood by engineers and tooling. It also doesn't have XML-specific additions like CDATA in script tags.
>
> That said, after talking through this again with Brendan we realized it may not end up being worth the effort for existing test files. If we were able to apply some of the incremental changes from step 5 to test .xhtml files that may be good enough. If the cost was low to include step 3 (like, if we had a tool that mostly automated away the process) then I’d prefer to do it. But it could be pragmatic to skip step 3 (at least for test documents). We’ll be working on some tooling anyway for step 5, so will spend a bit of time seeing how hard it looks to be to automate the html doc conversion.
>
> Brian
>
> > On May 14, 2019, at 12:37 PM, Boris Zbarsky <bzba...@mit.edu> wrote:
> >
Reply all
Reply to author
Forward
0 new messages