937896 - Screen orientation change does not change zoom to keep CSS scrollport width constant

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Botond Ballo [:botond]]

My understanding from a discussion I had with Kats today is that if you start viewing a page in portrait orientation, and the visible portion of the page is say 300 CSS pixels wide, then changing the orientation to landscape should increase the zoom such that the visible portion of the page remains 300 CSS pixels wide.

This does not currently happen on B2G - the zoom remains unchanged.

Comment 1

[Botond Ballo [:botond]]

The problem is that, while AsyncPanZoomController::UpdateCompositionBounds() updates the zoom as desired, but then it is changed back to the original at http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/AsyncPanZoomController.cpp#1292.

I don't really understand what the code at that line is doing. Removing the change to the zoom at that line solves the problem, but introduces some other problems with my dynamic toolbar implementation. I will continue to investigate.

Comment 2

[Botond Ballo [:botond]]

Attachment: bug937896.patch

Removing that zoom seems to fix the problem.

Note that zooming out is currently broken (bug 935219), and switching orientation from landscape -> portrait now causes a zoom-out (as it should), so with this patch, we run into bug 935219 more often than without. If that's concerning, we can hold off on landing this until bug 935219 is fixed (I am working on that next).

Try push: https://tbpl.mozilla.org/?tree=Try&rev=ce97bab3fea4

Comment 3

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

Comment on attachment 831860 [details] [diff] [review]
bug937896.patch

Review of attachment 831860 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/ipc/AsyncPanZoomController.cpp
@@ +1296,1 @@
>        needContentRepaint = true;

You can get rid of previousViewportWidth if you move the mFrameMetrics.mViewport assignment to after this if block.

Comment 4

[Botond Ballo [:botond]]

Attachment: bug937896.patch

Updated with Kats' comment. Carrying r+.

Comment 5

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #2)
> Try push: https://tbpl.mozilla.org/?tree=Try&rev=ce97bab3fea4

Try push is clean.

Comment 6

[Botond Ballo [:botond]]

http://hg.mozilla.org/integration/b2g-inbound/rev/9c35d632c11c
http://hg.mozilla.org/integration/b2g-inbound/rev/1369d5047f0f

Comment 7

[Botond Ballo [:botond]]

Attachment: bug937896-bustage.patch

(There was a compiler error in the first patch I pushed to b-i. This patch fixes it.)

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9c35d632c11c
https://hg.mozilla.org/mozilla-central/rev/1369d5047f0f