Last Comment Bug 694901 - (metaviewport) [birch] meta name="viewport" support for native Android front-end
(metaviewport)
: [birch] meta name="viewport" support for native Android front-end
Status: VERIFIED FIXED
: feature
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P1 normal (vote)
: Firefox 11
Assigned To: Matt Brubeck (:mbrubeck)
:
:
Mentors:
: 699006 703752 703767 704896 (view as bug list)
Depends on: 756473 703141 704950 706215 706309
Blocks: 699006
  Show dependency treegraph
 
Reported: 2011-10-16 21:38 PDT by :Ehsan Akhgari
Modified: 2016-07-29 14:19 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
WIP (7.13 KB, patch)
2011-11-03 08:41 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
WIP 2 (14.38 KB, patch)
2011-11-14 12:35 PST, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
WIP 3 (13.06 KB, patch)
2011-11-16 09:12 PST, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
WIP 4 (13.58 KB, patch)
2011-11-21 10:01 PST, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
WIP 5 (13.58 KB, patch)
2011-11-21 16:05 PST, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
WIP 6 (15.39 KB, patch)
2011-11-21 16:39 PST, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Splinter Review
patch (14.83 KB, patch)
2011-11-28 13:53 PST, Matt Brubeck (:mbrubeck)
mark.finkle: review+
Details | Diff | Splinter Review
2/2: set the default zoom level based on the viewport (1.94 KB, patch)
2011-11-29 10:11 PST, Matt Brubeck (:mbrubeck)
mbrubeck: review+
Details | Diff | Splinter Review

Description :Ehsan Akhgari 2011-10-16 21:38:45 PDT
I've been running the native UI for the past few days, and one of the problems I've been facing is that Google Reader appears much smaller in the native UI than in the XUL UI, making it barely readable.  I am on a Galaxy S Vibrant phone.
Comment 1 Matt Brubeck (:mbrubeck) 2011-10-17 07:14:05 PDT
The native front-end does not yet support the <meta name="viewport"> tag and related features to scale pages intelligently based on the device size and density.

We don't yet have real zooming implemented in birch; viewport sizing will depend on that.  (People are working on it already, but I don't have a bug number.)
Comment 2 Matt Brubeck (:mbrubeck) 2011-11-03 08:41:57 PDT
Created attachment 571657 [details] [diff] [review]
WIP

This work-in-progress patch doesn't do anything useful yet, but it transplants most of the viewport code from XUL Fennec and gets it running in native Fennec.
Comment 3 Matt Brubeck (:mbrubeck) 2011-11-14 12:35:41 PST
Created attachment 574378 [details] [diff] [review]
WIP 2

This is fully-functional, but has some bugs.  For example, the real screen/window size is not exposed to Gecko chrome, so "width=device-width" is currently using the wrong dimensions.

Next step is to integrate this with the displayport patch queue.
Comment 4 Matt Brubeck (:mbrubeck) 2011-11-16 09:12:44 PST
Created attachment 574914 [details] [diff] [review]
WIP 3

Updated to latest birch tip.  Still working out some bugs.
Comment 5 Kevin Brosnan [:kbrosnan] 2011-11-21 09:39:23 PST
*** Bug 703767 has been marked as a duplicate of this bug. ***
Comment 6 Matt Brubeck (:mbrubeck) 2011-11-21 10:01:50 PST
Created attachment 575897 [details] [diff] [review]
WIP 4

Rebased to birch tip.
Comment 7 Doug Turner (:dougt) 2011-11-21 13:55:16 PST
*** Bug 703752 has been marked as a duplicate of this bug. ***
Comment 8 Matt Brubeck (:mbrubeck) 2011-11-21 16:05:35 PST
Created attachment 576011 [details] [diff] [review]
WIP 5

Getting very close. This applies on top of the birch-pan-zoom patch queue [1], and it correctly sets the viewport size.  However, the gfx code still stores a viewport size based on the screen size, so the page's viewport gets overridden and reset on the next update from Java.

[1]: http://hg.mozilla.org/users/pwalton_mozilla.com/birch-pan-zoom/
Comment 9 Matt Brubeck (:mbrubeck) 2011-11-21 16:39:06 PST
Created attachment 576030 [details] [diff] [review]
WIP 6

This works; it sets and maintains the correct viewport.  Needs some testing, and I should talk to Chris about how to integrate the changes into his patches.
Comment 10 Kevin Brosnan [:kbrosnan] 2011-11-23 11:08:03 PST
*** Bug 704896 has been marked as a duplicate of this bug. ***
Comment 11 Patrick Walton (:pcwalton) 2011-11-24 20:54:29 PST
The patch applies cleanly, but doesn't seem to set the width to 980 by default for me.
Comment 12 Armen Zambrano [:armenzg] (EDT/UTC-4) 2011-11-25 08:22:28 PST
Did something land recently related to this work?

Sites now have a normal width!!! and I wanted to thank whoever did it.
Comment 13 Patrick Walton (:pcwalton) 2011-11-25 09:03:31 PST
(In reply to Armen Zambrano G. [:armenzg] - Release Engineer from comment #12)
> Did something land recently related to this work?
> 
> Sites now have a normal width!!! and I wanted to thank whoever did it.

This changed with the viewport patch. However, it's still wrong -- sites should be 980 by default, unless a <meta viewport> tag is present.
Comment 14 Matt Brubeck (:mbrubeck) 2011-11-28 13:53:51 PST
Created attachment 577359 [details] [diff] [review]
patch

This patch sets the viewport width and height correctly.

This does not yet set the correct initial scale; I think we might need to add a way to control zooming from JavaScript (or for the Java code to ask the JS code for an initial zoom level).
Comment 15 Matt Brubeck (:mbrubeck) 2011-11-28 14:17:20 PST
Bug 704950 might make the zooming part easier to fix.
Comment 16 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-28 21:36:33 PST
Comment on attachment 577359 [details] [diff] [review]
patch


>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>   startup: function startup() {

>+    ViewportHandler.init();

I didn't notice an uninit() call

>+  get defaultBrowserWidth() {
>+    var width = Services.prefs.getIntPref("browser.viewport.desktopWidth");

let

>+var ViewportHandler = {

>+  handleEvent: function handleEvent(aEvent) {

>+    let tab = BrowserApp.getTabForBrowser(browser);

Maybe I am paranoid, but we should probably check "tab" for null and bail
Comment 17 Matt Brubeck (:mbrubeck) 2011-11-29 10:11:08 PST
Created attachment 577652 [details] [diff] [review]
2/2: set the default zoom level based on the viewport

r=mfinkle (via IRC because Bugzilla was down)
Comment 19 Matt Brubeck (:mbrubeck) 2011-11-29 10:35:30 PST
*** Bug 699006 has been marked as a duplicate of this bug. ***
Comment 20 Aaron Train [:aaronmt] 2011-11-30 07:04:46 PST
Samsung Nexus S (Android 4.0.1)
20111130040240
http://hg.mozilla.org/projects/birch/rev/4e745f151abd

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