Last Comment Bug 715326 - Loading about:config initially via adb shell shows zoomed-in viewport
: Loading about:config initially via adb shell shows zoomed-in viewport
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P1 normal (vote)
: Firefox 12
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
: Sebastian Kaspari (:sebastian)
Mentors:
Depends on: 708746
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-04 13:56 PST by Kartikaya Gupta (email:kats@mozilla.com)
Modified: 2012-01-19 13:28 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
Logcat from about:fennec (23.54 KB, text/plain)
2012-01-04 13:56 PST, Kartikaya Gupta (email:kats@mozilla.com)
no flags Details
logcat from about:config (45.68 KB, text/plain)
2012-01-04 13:56 PST, Kartikaya Gupta (email:kats@mozilla.com)
no flags Details
(1/2) Don't round page size sending from JS to java (1.73 KB, patch)
2012-01-11 08:30 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
(2/2) Better focus when scaling up page smaller than viewport on both axes (2.88 KB, patch)
2012-01-11 08:32 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Kartikaya Gupta (email:kats@mozilla.com) 2012-01-04 13:56:05 PST
Created attachment 585881 [details]
Logcat from about:fennec

STR: (same as bug 714972)

1) adb shell am start -a android.intent.action.VIEW -n org.mozilla.fennec/.App about:fennec
2) adb shell am start -a android.intent.action.MAIN -c android.intent.category.HOME
3) adb shell /data/local/oom-fennec
4) adb shell am start -a android.intent.action.VIEW -n org.mozilla.fennec/.App about:config

On the first step about:fennec loads with an unexpected viewport.
Comment 1 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-04 13:56:51 PST
Created attachment 585882 [details]
logcat from about:config

After step 4 about:config renders with a perma-checkerboard and a bad viewport. Log attached.
Comment 2 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-11 07:36:26 PST
Bug 708746 fixed part of the problem here by initializing the java viewport page-size parameters to something more reasonable.
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-11 08:30:28 PST
Created attachment 587707 [details] [diff] [review]
(1/2) Don't round page size sending from JS to java
Comment 4 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-11 08:32:03 PST
Created attachment 587708 [details] [diff] [review]
(2/2) Better focus when scaling up page smaller than viewport on both axes
Comment 5 Patrick Walton (:pcwalton) 2012-01-12 18:13:22 PST
Comment on attachment 587708 [details] [diff] [review]
(2/2) Better focus when scaling up page smaller than viewport on both axes

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

Looks good, but I wonder if we should be zooming out to make page *height* (as opposed to page width) fill the screen at all. Seems like it's asking for trouble to do so, because page height tends to start very small when loading pages.
Comment 6 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-12 19:46:09 PST
(In reply to Patrick Walton (:pcwalton) from comment #5)
> Looks good, but I wonder if we should be zooming out to make page *height*
> (as opposed to page width) fill the screen at all. Seems like it's asking
> for trouble to do so, because page height tends to start very small when
> loading pages.

Good point. The code I added in bug 710297 might be simplified if we were to do that - we might not need to check the page size against the gScreen(Width|Height). I'm wondering if there are conditions where the page size can be forced to be smaller than viewport size though; if that's the case then the current approach is better since browser.js will send the page size update once the readyState flips over to complete.
Comment 9 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-19 09:30:16 PST
Comment on attachment 587707 [details] [diff] [review]
(1/2) Don't round page size sending from JS to java

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: size of rendered page could be off by up to a couple of pixels from where it should be
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low risk
Comment 10 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-19 09:31:59 PST
Comment on attachment 587708 [details] [diff] [review]
(2/2) Better focus when scaling up page smaller than viewport on both axes

[Approval Request Comment]
Regression caused by (bug #): none
User impact if declined: sometimes, on pages smaller than the viewport, the page will be scrolled to the end on one axis
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): possible misplacement of the page during zoom operations
Comment 11 Alex Keybl [:akeybl] 2012-01-19 12:03:41 PST
Comment on attachment 587707 [details] [diff] [review]
(1/2) Don't round page size sending from JS to java

[Triage Comment]
Mobile only - approved for Aurora.

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