Closed Bug 898140 Opened 7 years ago Closed 6 years ago

Move some mochitests that use XUL/XBL to their specific XUL/XBL test subdirectories


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: martijn.martijn, Assigned: martijn.martijn)



(1 file, 2 obsolete files)

There are a bunch of mochitests that use XUL/XBL that don't reside in content/xul/ or layout/xul/ or content/xbl/ subdirectories.
It seems to me that it would be best if those tests would move to those directories.

The reason is that XUL and XBL are Mozilla only technologies (which are going away, fwiu).  

Some files where this is the case (probably not all of them):

There's an advantage to keeping the tests near the code most likely to break the tests -- developers often run the "local" tests locally (e.g., I run layout/style/ mochitests frequently when changing any code in layout/style) for quick feedback before submitting patches to the try server.
Ok, fair enough.

I mainly thought about this in order to remove those entries from the b2g.json file (b2g mochitest currently don't support XUL/XBL).

Could I add some code to tests that use XBL that checks whether XBL is enabled/supported? 
I know to check for this, using SpecialPowers.hasPermission('allowXULXBL', document);
But is there perhaps a regular dom method that can check if XBL is working?

That would leave the 2 .xul files. I think those could be moved to chrome tests.
layout/base/tests/test_bug465448.xul could be moved to layout/base/tests/chrome/test_bug465448.xul
dom/tests/mochitest/general/test_focusrings.xul could be moved to dom/tests/mochitest/chrome/test_focusrings.xul

Is that ok?
Assignee: nobody → martijn.martijn
Worth noting that when you have *some* tests in separate subdirs according to criteria like "uses XUL", it implies an organization and leads you to believe that tests that aren't there don't meet the criteria. Rogues become more confusing.

I get David's point, but would be nice if at least the naming of the tests made that dependency more transparent. I'd have never assumed "content/base/test/test_bug372086.html " used XUL or XBL when there directories right there too.
The organization of the tests is more about their main focus.  A majority of our tests test the HTML parser in some way, but there's only a small set that focus on the HTML parser.
Attached patch test_xbl_userdata.diff (obsolete) — Splinter Review
Would something like this be acceptable for the tests that have xbl in them, that don't reside in content/xbl?
Attachment #784088 - Flags: feedback?
That doesn't seem great; if that test fails, we'll just stop running any tests.

What's the rationale for this?  In other words, why is it worth spending time on rather than other things?

I think if we want to write cross-browser tests, they should be using testharness.js, not mochitest.
To support gecko browsers that don't support XBL, I guess (which is actually the case for Firefox itself, except in mochitest it's enabled globally).
Attached patch move_xul_to_chrome.diff (obsolete) — Splinter Review
Ok, I'll leave the xbl tests as they are.

But is changing the xul tests to mochitest-chrome ok? Fennec and b2g don't support xul and these seem one of the few (only?) mochitests that are still xul based.
I also changed test_selection_expanding.html to mochi-test chrome, because this is also working for desktop only.
Attachment #784088 - Attachment is obsolete: true
Attachment #784088 - Flags: feedback?
Attachment #784712 - Flags: feedback?(dbaron)
With removal in b2g.json, android.json.
Attachment #784712 - Attachment is obsolete: true
Attachment #784712 - Flags: feedback?(dbaron)
Attachment #784943 - Flags: feedback?(dbaron)
Closed: 6 years ago
Resolution: --- → INVALID
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.