1619169 - Planet mozilla content gets visibly clipped when dynamic toolbar is hidden on WebRender

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

The clip is applied here, and here. The latter part is causing the top content is clipped.

I don't know why this issue doesn't happen on non-WebRender.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

So a problem is in nsLayoutUtils::ExpandHeightForViewportUnits.

On my local Android Emulator, the sizeForViewportUnits is (980px, 1428px) (in CSS units) and the pres context's visible area is (980px, 1372px) (in CSS units) because there is no meta viewport specified. BUT the display size on the device (obtained by MobileViewporrManager::DisplaySize is (800, 1120), which means the content is slightly scaled down. But unfortunately we use the dynamic toolbar maximum height as it as.

So presumably this planet mozilla case will be fixed by bug 1641166, but it's not yet clear about the exact condition when we tweak the vh height afected by the dynamic toolbar maximum height as I raised a question in bug 1641166 comment 0.

From what I can tell is that we should use the display size's height and the dynamic toolbar maximum height directly to calculate the ratio we need to expand something like;

dynamic toolbar max height / display height * given size's height + given size's height

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: A reftest

FWIW, here is a reftest I tried to replicate this issue but it doesn't actually. Probably specifying reftest-displayport-h obscure the problem.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Reassigning per request on Matrix. You definitely seem to have a better handle on this than I do, and I don't have cycles to look into this in the near future.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1619169 - Duplicate ExpandHeightForViewportUnits to ExpandHeightForDynamicToolbar. r?botond

And use the duplicated one at the places where we need the expanded size for
interactions with the dynamic toolbar on the compositor. The new function will
be modified in the next commit.

Note that the only one remaining call site of ExpandHeightForViewportUnits is
for window.inner{Width,Height}. For window.inner{Width,Height} we don't yet
return the layout viewport (which might be expanded by the minimum-scale), it's
going to be fixed in bug 1598487 [1], but it's not ready to fix because there
also need fixes in comm-central (see dependencies in the bug). So for now, we
should keep the current behavior for window.inner{Width,Height}.

Also note that it's not yet clear whether we will eventually replace the last
call site of ExpandHeightForViewportUnits with ExpandHeightForDynamicToolbar
since the value corresponding to the dynamic toolbar might NOT be affected by
the minimum-scale in some cases. See bug 1641166 for details.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1598487
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1641166

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1619169 - Expand the given size with the ratio of the dynamic toolbar max height to the display size in ExpandHeightForDynamicToolbar. r?botond

In some cases the visible area has been already scaled to a certain scale
visually to fit the content to the display, whereas we apply the max height of
the dynamic toolbar to the visible area as it is [1], which means the
resolution of the each value mismatches. Ideally this mismatch should be fixed
by factoring the resolution differece, but there are some edge cases we can't
simply fix it as I described in bug 1641166.

So, here we take a different approach which is not affected by the content
visible area's resolution value.

[1] https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/layout/base/nsPresContext.cpp#2527
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1641166

Depends on D78440

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2e402b376f9
Duplicate ExpandHeightForViewportUnits to ExpandHeightForDynamicToolbar. r=botond
https://hg.mozilla.org/integration/autoland/rev/6237102f005d
Expand the given size with the ratio of the dynamic toolbar max height to the display size in ExpandHeightForDynamicToolbar. r=botond

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/b2e402b376f9
https://hg.mozilla.org/mozilla-central/rev/6237102f005d