Avoid using the visual viewport for laying out scrollbars
Categories
(Core :: Layout: Scrolling and Overflow, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox80 | --- | fixed |
People
(Reporter: kats, Assigned: kats)
References
(Depends on 1 open bug)
Details
Attachments
(2 files)
The visual viewport size depends on the presence or absence of root scrollframe's scrollbars. So if we have scrollbar layout depending on the visual viewport size that's conceptually a circular dependency. While the dependency can be resolved in a couple of cycles because it converges, it's still not ideal because it means we need at least two reflows to lay out a page. I was seeing this while investigating some reftest failures with WIP patches on bug 1647034.
I think the correct thing to do here is to not read the visual viewport size from the presShell while laying out the scrollbars. If we need the composition size we can get that directly from nsLayoutUtils or other places. And then after doing the scrollbar layout we can set the visual viewport size on the presShell based on the presence of scrollbars. This way we can get away with reaching a correct/consistent state on the first reflow.
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Comment 2•5 years ago
|
||
Looks like there is a failure in browser_ext_browserAction_popup_resize.js. Based on initial local debugging I thought Emilio's recent patches to have a single source of truth for composition size would fix it but it doesn't appear to, unfortunately. I will have to debug more.
|
|
Assignee | |
Comment 3•5 years ago
|
||
So I tried debugging this failure a bit. It's a bit special because it's a popup window and therefore doesn't have APZ or an MVM at all. And the way the layouting of it seems to work is that the test sets the body size and then magically expects the window's innerWidth/height to expand to reflect that. Which I guess is somehow special popup behaviour, because regular windows don't resize themselves based on their content size. I'm not really sure how that works, I tried tracing backwards with rr but I don't have a good overall understanding of where to look and all my stacks were just deep in reflows and not very illuminating.
Anyway, in theory when the test sets the body size, it reflows the document inside the popup while the popup is still at the "old" size (say 0x0 for the first sub-test). And in that scenario the popup gets scrollbars, because its content is larger than 0x0, and that causes the window size to end up bigger than what the test is expecting. A later reflow removes the scrollbar now that the window is bigger, but it doesn't shrink the window back down. So the test ends up failing.
This seems like something we should handle properly, but for now I'm going to cheat and just check for the presShell having an MVM because that will fix this failing test while still accomplishing the goal of this bug. I can spin out another follow-up bug to investigate this more but I don't plan to work on it because too much yak-shaving for me.
|
|
Assignee | |
Comment 4•5 years ago
|
||
|
|
Assignee | |
Comment 5•5 years ago
|
||
The TryLayout code uses the composition size to figure out if the
scrollbar is needed or not. It computes something similar to the visual
viewport size (but assuming no scrollbars are taking up space) to do this.
There's no reason this code should be depending on whether or not the
visual viewport size is set in the presShell, so we can just remove the
condition. That way even on the very first reflow, when the visual viewport
will definitely not be set, we can correctly determine if the scrollbars
need to be laid out or not.
|
|
Assignee | |
Comment 6•5 years ago
|
||
When positioning overlay scrollbars, the code was previously taking the
visual viewport size and scaling it back up to the composition size using
the resolution. This works because overlay scrollbars don't take up any
space, and so the visual viewport size is exactly equal to the composition
size divided by the resolution. However, it's simpler to just use the
composition size because we can get that easily enough. And this fixes the
scrollbar positioning on the very first reflow, before the visual viewport
has been set on the presShell.
Eventually this might make it easier to do this for non-overlay scrollbars
as well, since those do take up layout space, and can't be positioned using
the "VV size multiplied by resolution" quantity, as that doesn't exactly
equal the composition size.
Depends on D81276
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/4a9a1d126912
https://hg.mozilla.org/mozilla-central/rev/e420bc112b10
Description
•