Closed Bug 1187792 Opened 6 years ago Closed 6 years ago
[APZ] Resize event handler may get the old window inner size when there is scrollbar
Steps to reproduce: 1. Open the attachment 2 [review]. Maximum the window 3. Restore the window The first size in each item is the window inner size, the second is the outer size. Expected result: The inner size should always match the outer size (only with the difference of the chrome), and reflect the size after the resize Actual result: The inner size remains the old size when the handler is called This happens on both e10s and non-e10s. Without scrollbar (removing the super tall <div>), on e10s, this could still be reproduced when restoring the window. It should be a recent regression.
This could cause extra reflow which slows down the fullscreen change.
Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=738d8ee106fe&tochange=af67b5f73381 Triggered by: Bug 1157745 Regression window( with force apz enabled ): https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=49ae0961591e&tochange=fee45cd9a4d9 Regressed by: Bug 1178847
Thanks for the regression window. Kartikaya, could you have a look at this issue?
Interesting. Yeah I can take a look.
Assignee: nobody → bugmail.mozilla
I can reproduce bad window.innerWidth/innerHeight values on Linux with APZ enabled. I think the problem is that window.innerWidth/innerHeight return the size of the scroll position clamping scroll port size if one is set . In the case that there is no MobileViewportManager - such as on desktop - that rect doesn't get updated until after an APZ repaint, which happens after the resize event is fired to content. This shouldn't affect B2G since there we do have a MobileViewportManager which updates the SPCSPS before the resize event is fired to content. Not 100% sure what the correct fix here is, but I think not setting the SPCSPS on desktop (in APZCCallbackHelper::UpdateRootFrame) is probably a good thing to do. That by itself will fix this bug but we may want to make a few other tweaks as well.  http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp?rev=6a28170cf9a2#4869
Component: DOM: Core & HTML → Panning and Zooming
Summary: Resize event handler may get the old window inner size when there is scrollbar → [APZ] Resize event handler may get the old window inner size when there is scrollbar
I think this is probably ok for now. My reasoning for the MVM condition change is that if we have a desktop platform which supports zooming (e.g. desktop windows with pinch-zoom) then we want to have the MVM active, but MetaViewportEnabled() still false. Thoughts?
Attachment #8639385 - Flags: review?(botond)
Comment on attachment 8639385 [details] [diff] [review] Patch Review of attachment 8639385 [details] [diff] [review]: ----------------------------------------------------------------- Looks reasonable to me! Naming-wise, though, is it still a *mobile* viewport manager if it's used for desktop-with-zooming?
Attachment #8639385 - Flags: review?(botond) → review+
No I guess not.. :( Naming things is hard. Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=64366b523e6c, waiting for the results before landing.
Depends on: 1197592
You need to log in before you can comment on or make changes to this bug.