It should be pretty much good after bug 1582316 and bug 1581973, but I'll make a try push with the assertion patch to make sure.

Thanks for filing!

(RE "turn it on by default or remove the pref" - I'd advocate for just turning it on, via the .yaml file, so that we've still got a quick and easy way to roll back if needed, and to locally test with/without this behavior if we think we might need to roll back).

I'm in meetings for the next few hours but I'll gladly r+ a patch assuming this doesn't break anything (thanks for doing a try push).

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ff0933bdaa79a63e0e9885165e366099bf2ca64

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f356854af861dd7cf366d71c2c4b868717589c95

We discussed and are going to aim for flipping the pref in this bug to give it some time to bake on nightly before the pref gets removed.

Depends on D46675

This is not quite ready for review, there are two remaining crashes AFAIK: the mozscreenshots related one and the tab overflow test one.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1097341e3d896dad16789fd1105b74208761697c

This change to 360339-2.xul is problematic and will probably cause e.g. the <style> element itself to show up visibly on the page. (Normally it's display:none.)

<html:style>
-* { float: right; }
+* { float: right; display: block; }

https://hg.mozilla.org/integration/autoland/rev/0d8d8016da4f#l2.13

Please add one more followup, to instead apply that display:block in a more targeted way (to just the elements that matter), like we did for the -1 version of the crashtest here:
https://hg.mozilla.org/integration/autoland/rev/deba67add7d2#l6.2

Backed out 3 changesets (Bug 1582530) for crashtest failures on 360339-1.xul.

Backout: https://hg.mozilla.org/integration/autoland/rev/fb10058593f7ec2146afd9fc8844eec900e921ae

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=0d8d8016da4f0066e4ddec33f52d0b5669a46060&selectedJob=268642349

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268642349&repo=autoland&lineNumber=30941

Backed out 2 changesets (Bug 1582530) for crashtest assertions on ReflowInput.cpp.

Backout: https://hg.mozilla.org/integration/autoland/rev/3521a3b25d32e7ac6c8d1670378b452953ddfd3e

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=1ae40ac76cd0a1fe5fda7dbf056025d21c71d7b9&selectedJob=268700146

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268700146&repo=autoland&lineNumber=10943

Mochitest failure log (2):
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268702866&repo=autoland&lineNumber=12726

Backed out 2 changesets (Bug 1582530) for causing reftest permafailures in /builds/worker/workspace/build/src/layout/generic/ReflowInput.cpp:2188 CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=268705816

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268705816&repo=autoland&lineNumber=38922

(In reply to Cristina Coroiu [:ccoroiu] from comment #16)

Mochitest failure log (2):
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268702866&repo=autoland&lineNumber=12726

Hi Daniel, Is the fullscreen assertion failure something you could look at ? I couldn't figure out what element is affected in this specific case.

Thanks :)

I haven't been able to reproduce that assertion failure during dom/html/test/test_fullscreen-api.html in my local Linux environment, but perhaps that's not surprising because it looks like the affected push only shows that issue happening on Windows and Mac. (There are several linux platforms with all-green M-1 through e.g. M-16` runs.)

I've adjusted the assertion to move it a bit later (after the frame's been added to the frame tree, so that it can be dumped), and I added logging to print the frame's address and dump the frame tree, and I've pushed that to Try (as a child of the exact commit for comment 18's failure):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=622426a82abd49f2fbc11d7602d148d70a84a713

Hopefully it crashes for the same frame and the diagnostic info is useful for tracking it down...

Built locally on Windows and poked around a bit. So that fullscreen-api failure is happening for a XUL element that is the child of an HTML body, and whose tag-name is literally "element".

I'm pretty sure it's this "element" here, since this is the testcase that we're running when the failure happens:

function testNamespaces(followupTestFn) {
  let tests = [
  [omitting some lines here for brevity]
    {allowed: true,  name: "element", ns: "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"},

https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/dom/html/test/file_fullscreen-api.html#292

It looks like the test is validating that a XUL element can be the document.fullscreenElement (among other things).

And fullscreening an element makes the element fixed-pos, via this line:

*|*:fullscreen:not(:root) {
  position: fixed !important;

https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/layout/style/res/ua.css#330

So whenever a XUL element becomes fullscreen, we also need to give it "display:block" in order for it to actually behave as a proper fixedpos . :-/

Obvious question: do we even use the fullscreen APIs with XUL, in practice? If we could prove that we don't, that would be helpful to let us e.g. remove this part of the testcase and not worry about it.

(In reply to Daniel Holbert [:dholbert] from comment #22)

It looks like the test is validating that a XUL element can be the document.fullscreenElement (among other things).

And fullscreening an element makes the element fixed-pos, via this line:

*|*:fullscreen:not(:root) {
  position: fixed !important;

https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/layout/style/res/ua.css#330

So whenever a XUL element becomes fullscreen, we also need to give it "display:block" in order for it to actually behave as a proper fixedpos . :-/

Thanks for looking into it!

I've added:

xul|*:fullscreen:not(:root) {
  /* The position: fixed; property above used to force the computed display
   * value to block. It is no longer the case now, so we manually set it here to
   * maintain the old behaviour. */
  display: block;
}

but I think that might not fix the cases where we explicitly set display: -moz-box and use fullscreen at the same time (if we ever do), unless that display: block; has !important, which might be problematic in other cases (hidden element for example), though I'm not sure a hidden element can actually get the fullscreen state.

Obvious question: do we even use the fullscreen APIs with XUL, in practice? If we could prove that we don't, that would be helpful to let us e.g. remove this part of the testcase and not worry about it.

Bug 1305928 comment 1 says it's needed but I don't actually know if that's still true. Thomas, do you remember where this was needed ?

ntim: I just remember a test failing without that, not sure if it's still necessary.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=68b702ed7f9eb0e946dae20974b6e64b3f324d39

Backed out 2 changesets (bug 1582530) for Creshtest failures in ayout/generic/ReflowInput.cpp. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268853036&repo=autoland&lineNumber=38922

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=536e78fea3de622d967f5de4b5d8fd4b20abc878

Backout:
https://hg.mozilla.org/integration/autoland/rev/8ce8534b2d857004d83198795b3f56ef6ab86568

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c3d7b0d23e14b317dae96b91702cec75d2f8e0e

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8936826eccc4616dcb0665ff7c2ce3c47e6ed1e3

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfbc4c7494ee20b9c3a0a3ca561d0b1fafdb4f3b

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0eba08723ec8f74e2ab622f0d7cc7adc5275ba45

https://hg.mozilla.org/mozilla-central/rev/b768fb9e4983
https://hg.mozilla.org/mozilla-central/rev/72a8d8c20180