Titlebar doesn't get collapsed on OSX when running in browser.xhtml due to MozBeforeInitialXULLayout not firing

RESOLVED FIXED in Firefox 63

Status

()

defect
P1
normal
RESOLVED FIXED
9 months ago
6 months ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

unspecified
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

9 months ago
This is because onBeforeInitialXULLayout doesn't get called, since the MozBeforeInitialXULLayout event never fires. A couple of options:

1) Fire a new event like MozBeforeInitialTopLevelLayout from nsXULWindow::BeforeStartLayout and then update browser.xul to listen to that. I believe we need to keep MozBeforeInitialXULLayout so Fluent can continue to listen to it in non-top-level XUL documents.
2) Change the code listening in browser.js so that we listen for DOMContentLoaded in HTML and MozBeforeInitialXULLayout in XUL. This appears at first glance to be fine functionally, but likely has performance issues.
(Assignee)

Comment 1

8 months ago
Just an update here: for some reason MozBeforeInitialXULLayout (and/or MozBeforeInitialTopLevelLayout if we expose it for all top level docs) actually fires before the inline script that listens to it in browser.js runs, so listening to it from the HTML document doesn't work.
(Assignee)

Updated

8 months ago
Duplicate of this bug: 1484081
(Assignee)

Updated

8 months ago
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
(Assignee)

Comment 3

8 months ago
This is an extension of the work in Bug 1473160 to make clear in what environments
various gBrowserInit functions are run. Since we currently use these in an `if` block
in browser.js for "navigator:browser" window types, and browser.xul is the only
navigator:browser window that loads browser.js, this moves the event listeners directly
into browser.xul so it's extra clear that they don't run in non-browser top level windows on OSX.

Also move a few on-event handlers from the <window> tag into this block so they
all happen in one place.
(Assignee)

Comment 4

8 months ago
MozBeforeInitialXULLayout doesn't fire in HTML docs. Even if we change it to
start firing for all top-level docs, it ends up firing _before_ the inline script
runs inside of browser.xul. For now, run that logic in DOMContentLoaded to
at least get a more functional browser window.
(Assignee)

Comment 5

8 months ago
Gijs, I've moved the listeners directly into browser.xul but would also be happy to move them into browser.js (and then do a runtime check on document.contentType to determine if we are the xhtml browser). The reason I chose the former is that I figured browser.xul was a bit more explicit that it wouldn't be running in non-browser windows, but either place is fine for me.
(Assignee)

Comment 6

8 months ago
This also moves the onload/onunload/onclose handlers from the <window> tag to the block and migrates them to use addEventListener, which AFAICT won't be a problem (I don't see any code that overrides window.onload, for instance). Can also punt on either of those changes if you'd prefer.
(Assignee)

Comment 7

8 months ago
Here's a version that makes minimal changes and also fixes the bug. If you prefer how things are set up now to how Part 1 changes things, we can go with something like this instead.
Priority: -- → P1

Comment 8

8 months ago
Comment on attachment 9003564 [details]
Bug 1482448 - Part 1 - Move window listeners for browser.xul into one place;r=Gijs

:Gijs (he/him) has approved the revision.
Attachment #9003564 - Flags: review+

Comment 9

8 months ago
Comment on attachment 9003565 [details]
Bug 1482448 - Part 2 - Run gBrowserInit.onBeforeInitialXULLayout in browser.xhtml;r=Gijs

:Gijs (he/him) has approved the revision.
Attachment #9003565 - Flags: review+

Comment 10

8 months ago
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71d8a7a05ee0
Part 1 - Move window listeners for browser.xul into one place;r=Gijs

Comment 11

8 months ago
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/036bcf5f704b
Part 2 - Run gBrowserInit.onBeforeInitialXULLayout in browser.xhtml;r=Gijs

Comment 12

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/71d8a7a05ee0
https://hg.mozilla.org/mozilla-central/rev/036bcf5f704b
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
(Assignee)

Updated

7 months ago
See Also: → 1497599
(Assignee)

Updated

6 months ago
See Also: → 1497975
You need to log in before you can comment on or make changes to this bug.