If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED INVALID

Status

()

Core
DOM
RESOLVED INVALID
4 years ago
3 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: Martijn Wargers (dead))

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
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):
layout/style/test/media_queries_dynamic_xbl_iframe.html
layout/style/test/test_media_queries_dynamic_xbl.html 
content/base/test/test_xbl_userdata.xhtml 
content/base/test/test_base.xhtml 
content/base/test/test_bug330925.xhtml 
content/base/test/test_bug372086.html 
content/base/test/test_bug419527.xhtml 
content/base/test/test_bug444030.xhtml 
content/events/test/test_bug391568.xhtml 
layout/inspector/tests/test_bug522601.xhtml 
layout/style/test/test_selectors_on_anonymous_content.html
dom/tests/mochitest/general/test_focusrings.xul
layout/base/tests/test_bug465448.xul

Agree/disagree?
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.
(Assignee)

Comment 2

4 years ago
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)

Updated

4 years ago
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.
(Assignee)

Comment 5

4 years ago
Created attachment 784088 [details] [diff] [review]
test_xbl_userdata.diff

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.
(Assignee)

Comment 7

4 years ago
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).
(Assignee)

Comment 8

4 years ago
Created attachment 784712 [details] [diff] [review]
move_xul_to_chrome.diff

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)
(Assignee)

Comment 9

4 years ago
Created attachment 784943 [details] [diff] [review]
move_xul_to_chrome.diff

With removal in b2g.json, android.json.
Attachment #784712 - Attachment is obsolete: true
Attachment #784712 - Flags: feedback?(dbaron)
Attachment #784943 - Flags: feedback?(dbaron)
(Assignee)

Updated

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