Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gecko Bug 1935189] Don't make frame initial viewport size depend on whether it's been laid out. #49955

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

moz-wptsync-bot
Copy link
Collaborator

This is rather tricky / unfortunate, but this is the low-risk fix for
now. The issue is as follows:

When we construct a subdocument frame, we try to show its viewer by
using a script runner which will call ShowViewer().

If ShowViewer() gets called before the subdoc frame gets laid out
(has the NS_FRAME_FIRST_REFLOW bit set), we pick an (arbitrary) 10x10
device pixel size, and initialize the page with that viewport.

Then, eventually, the frame gets laid out to the correct size and fires
a resize event.

If we have container queries around, we need to lay out the page while
styling, which causes the layout to happen before the script blocker
checkpoint, and thus the viewport to get initialized with the right size
to begin with.

This generally shouldn't be bad, but google chat at least seems to rely
on this resize event (and the divergence being caused by container
queries is rather unfortunate).

For now, do the low risk fix of always using the "initial" subdoc frame
size to show the viewer.

We should ideally sort this out better. Even our "good" behavior is
quite bizarre (e.g, I'd expect going from display: block to none to
trigger a resize event and the viewport to go back to 0... As you noted,
chrome's inner layout sizes do change when the frame is display: none).

But it seems worth to land this at least in the interim.

Differential Revision: https://phabricator.services.mozilla.com/D233399

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1935189
gecko-commit: 0c612ed5d669c157a34c24d0c7df4b7f8ccd5fe4
gecko-reviewers: dholbert

…id out.

This is rather tricky / unfortunate, but this is the low-risk fix for
now. The issue is as follows:

When we construct a subdocument frame, we try to show its viewer by
using a script runner which will call ShowViewer().

If ShowViewer() gets called before the subdoc frame gets laid out
(has the NS_FRAME_FIRST_REFLOW bit set), we pick an (arbitrary) 10x10
device pixel size, and initialize the page with that viewport.

Then, eventually, the frame gets laid out to the correct size and fires
a resize event.

If we have container queries around, we need to lay out the page while
styling, which causes the layout to happen before the script blocker
checkpoint, and thus the viewport to get initialized with the right size
to begin with.

This generally shouldn't be bad, but google chat at least seems to rely
on this resize event (and the divergence being caused by container
queries is rather unfortunate).

For now, do the low risk fix of always using the "initial" subdoc frame
size to show the viewer.

We should ideally sort this out better. Even our "good" behavior is
quite bizarre (e.g, I'd expect going from display: block to none to
trigger a resize event and the viewport to go back to 0... As you noted,
chrome's inner layout sizes do change when the frame is display: none).

But it seems worth to land this at least in the interim.

Differential Revision: https://phabricator.services.mozilla.com/D233399

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1935189
gecko-commit: 0c612ed5d669c157a34c24d0c7df4b7f8ccd5fe4
gecko-reviewers: dholbert
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Firefox project.

@moz-wptsync-bot moz-wptsync-bot merged commit 3567a89 into master Jan 8, 2025
19 checks passed
@moz-wptsync-bot moz-wptsync-bot deleted the gecko/1935189 branch January 8, 2025 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants