Last Comment Bug 704738 - Page/content does not resize on device rotation/orientation change
: Page/content does not resize on device rotation/orientation change
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P1 normal (vote)
: ---
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
: Sebastian Kaspari (:sebastian)
Mentors:
: 705094 707705 707934 (view as bug list)
Depends on: 707996
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-22 20:50 PST by Lawrence Mandel [:lmandel] (use needinfo)
Modified: 2016-07-29 14:20 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
Screenshot of portrait mode with grey area (43.18 KB, image/png)
2011-11-22 20:50 PST, Lawrence Mandel [:lmandel] (use needinfo)
no flags Details
Snap edge on rotate (1.11 KB, patch)
2011-12-05 10:16 PST, Kartikaya Gupta (email:kats@mozilla.com)
chrislord.net: review+
Details | Diff | Splinter Review
Re-zoom on rotate/viewport change (4.48 KB, patch)
2011-12-05 13:32 PST, Kartikaya Gupta (email:kats@mozilla.com)
wjohnston2000: review+
chrislord.net: feedback+
Details | Diff | Splinter Review
Patch (4.30 KB, patch)
2011-12-12 07:57 PST, Kartikaya Gupta (email:kats@mozilla.com)
chrislord.net: review+
Details | Diff | Splinter Review

Description Lawrence Mandel [:lmandel] (use needinfo) 2011-11-22 20:50:26 PST
Created attachment 576396 [details]
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 1 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-05 09:19:18 PST
*** Bug 707705 has been marked as a duplicate of this bug. ***
Comment 2 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-05 09:20:12 PST
*** Bug 705094 has been marked as a duplicate of this bug. ***
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-05 10:16:43 PST
Created attachment 579113 [details] [diff] [review]
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] 2011-12-05 10:21:45 PST
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:kats@mozilla.com) 2011-12-05 10:29:11 PST
https://hg.mozilla.org/projects/birch/rev/bea2748292cc

Leaving this bug open until bug 701594 is resolved.
Comment 6 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-05 13:32:35 PST
Created attachment 579146 [details] [diff] [review]
Re-zoom on rotate/viewport change
Comment 7 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-05 13:34:12 PST
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] 2011-12-06 02:13:46 PST
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) 2011-12-06 07:28:56 PST
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:kats@mozilla.com) 2011-12-06 07:53:08 PST
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 11 Aaron Train [:aaronmt] 2011-12-06 07:58:28 PST
*** Bug 707934 has been marked as a duplicate of this bug. ***
Comment 12 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-06 10:47:13 PST
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:kats@mozilla.com) 2011-12-09 14:12:10 PST
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] 2011-12-11 07:01:04 PST
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:kats@mozilla.com) 2011-12-12 07:27:07 PST
(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:kats@mozilla.com) 2011-12-12 07:57:33 PST
Created attachment 580909 [details] [diff] [review]
Patch

Just renaming variables and ensuring rotations interact well with double-tap.
Comment 17 Chris Lord [:cwiiis] 2011-12-12 08:09:41 PST
Comment on attachment 580909 [details] [diff] [review]
Patch

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

Looks fine to me.
Comment 18 Kartikaya Gupta (email:kats@mozilla.com) 2011-12-12 10:55:32 PST
https://hg.mozilla.org/mozilla-central/rev/6992abaa6854
Comment 19 Aaron Train [:aaronmt] 2011-12-13 06:55:12 PST
HTC Nexus One (Android 2.3.6)
20111213061518
http://hg.mozilla.org/mozilla-central/rev/e79b3396889c

Note You need to log in before you can comment on or make changes to this bug.