704738 - Page/content does not resize on device rotation/orientation change

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Lawrence Mandel [:lmandel] (use needinfo)]

Attachment: Screenshot of portrait mode with grey area

Open a web page in landscape mode and scroll all the way to the bottom. (I was on http://www.mikealrogers.com/posts/apache-considered-harmful.html when I found this bug.) Change to portrait mode. The page does not grab the bottom but instead shows a grey area on the bottom half of the screen.

Comment 3

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

Attachment: Snap edge on rotate

This fixes the snapping-to-edge on rotation. For making the page resize so we don't get grey areas, bug 701594 will need to be fixed first.

Comment 4

[Chris Lord [:cwiiis]]

Comment on attachment 579113 [details] [diff] [review]
Snap edge on rotate

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

Looks fine to me.

Comment 5

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

https://hg.mozilla.org/projects/birch/rev/bea2748292cc

Leaving this bug open until bug 701594 is resolved.

Comment 6

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

Attachment: Re-zoom on rotate/viewport change

Comment 7

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

Getting rid of the dependency on bug 701594 because https://hg.mozilla.org/projects/birch/rev/f8c174b95c40 (bug 697701) also provides the necessary zoom code. Use that to re-zoom the page on rotate if needed. This also handles the case where the device is rotated or the page calls scrollTo while in the middle of an animated zoom.

Comment 8

[Chris Lord [:cwiiis]]

Comment on attachment 579146 [details] [diff] [review]
Re-zoom on rotate/viewport change

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

Looks good, feedback+ from me.

::: mobile/android/base/ui/PanZoomController.java
@@ +185,5 @@
>          default:                        return false;
>          }
>      }
>  
>      public void geometryChanged(boolean aAbortFling) {

Maybe this should be renamed aAbortAnimation now?

Comment 9

[Wesley Johnston (:wesj)]

Comment on attachment 579146 [details] [diff] [review]
Re-zoom on rotate/viewport change

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

::: mobile/android/base/ui/PanZoomController.java
@@ +205,5 @@
>                  mX.velocity = mY.velocity = 0.0f;
>                  mState = PanZoomState.NOTHING;
>                  // fall through
>              case NOTHING:
> +                tryZoomToFitPage();

This will zoom us out right? I guess that's better than showing gray areas.

Comment 10

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

Renamed aAbortFling to abortAnimation. And yes, it zooms us out only if there would be gray areas otherwise.

https://hg.mozilla.org/projects/birch/rev/884b705c99fb

Comment 12

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

Backed out in https://hg.mozilla.org/projects/birch/rev/62a0c96c6ef7 since it broke some behaviour relating to opening new tabs.

Comment 13

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

Technically this works now because of the patches from bug 701594 but it could use a little tweaking. I'll put together a patch for it.

Comment 14

[Bill Gianopoulos [:WG9s]]

Well, one issue still remaining is that if you open a page in landscape, and then rotate to portrait triggering a zoom, rotating back does not zoom back.

This might be desired behavior, but seem counter intuitive to me.

Comment 15

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

(In reply to Bill Gianopoulos from comment #14)
> Well, one issue still remaining is that if you open a page in landscape, and
> then rotate to portrait triggering a zoom, rotating back does not zoom back.
> 
> This might be desired behavior, but seem counter intuitive to me.

I agree that this is annoying. It's being tracked in bug 707956.

Comment 16

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

Attachment: Patch

Just renaming variables and ensuring rotations interact well with double-tap.

Comment 17

[Chris Lord [:cwiiis]]

Comment on attachment 580909 [details] [diff] [review]
Patch

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

Looks fine to me.

Comment 18

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

https://hg.mozilla.org/mozilla-central/rev/6992abaa6854

Comment 19

[Aaron Train [:aaronmt]]

HTC Nexus One (Android 2.3.6)
20111213061518
http://hg.mozilla.org/mozilla-central/rev/e79b3396889c