556619 - portrait mode - about:config rows shorter than a touchheight

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Madhava Enros [:madhava]]

The layout is fine in landscape, but is scaled down to less than a touchheight in portrait:

http://www.flickr.com/photos/madhava_work/4482700656/

Comment 1

[Madhava Enros [:madhava]]

I suspect that this will be the case for the tabs from other computers page too

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

We should disable zoom for XUL pages, imo

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Overall I liked the patch but had some refactoring I'd like to see.

I made a patch that builds on yours

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch 2

This patch builds on the first. It moves some of the logic into isContentHandheld so we can re-use it at other times.

It also cleans up isContentHandheld so we don't need to test some many different "reasons" in other code. All modes return "scale" and I add "allowZoom" which will be used in later patches.

The patch removes the special "browser-chrome" which was a duplicate of "window-width" and "window-height"

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 438532 [details] [diff] [review]
patch 2


>   getPageZoomLevel: function getPageZoomLevel() {
>diff --git a/chrome/content/Util.js b/chrome/content/Util.js
>--- a/chrome/content/Util.js
>+++ b/chrome/content/Util.js
>@@ -111,24 +111,27 @@ let Util = {
>   makeURLAbsolute: function makeURLAbsolute(base, url) {
>     // Note:  makeURI() will throw if url is not a valid URI
>     return makeURI(url, null, makeURI(base)).spec;
>   },
> 
>   contentIsHandheld: function contentIsHandheld(browser) {
>     let doctype = browser.contentDocument.doctype;
>     if (doctype && /(WAP|WML|Mobile)/.test(doctype.publicId))
>-      return {reason: "doctype", result: true};
>+      return { reason: "doctype", result: true, scale: 1.0 };
> 
>     let windowUtils = browser.contentWindow
>                              .QueryInterface(Ci.nsIInterfaceRequestor)
>                              .getInterface(Ci.nsIDOMWindowUtils);
>     let handheldFriendly = windowUtils.getDocumentMetadata("HandheldFriendly");
>     if (handheldFriendly == "true")
>-      return {reason: "handheld", result: true};
>+      return { reason: "handheld", result: true, scale: 1.0 };
...
>     if (viewportScale > 0 || viewportWidth > 0 || viewportHeight > 0) {
>       return {
>         reason: "viewport",
>         result: true,
>         scale: viewportScale,
>         width: viewportWidth,
>         height: viewportHeight,
>-        autoSize: viewportWidthStr == "device-width" || viewportHeightStr == "device-height"
>+        autoSize: viewportWidthStr == "device-width" || viewportHeightStr == "device-height",
>+        allowZoom: windowUtils.getDocumentMetadata("viewport-user-scalable") != "no"
>       }
>     }
> 
>-    return {reason: "", result: false};
>+    return { reason: "", result: false, allowZoom: true };
>   },

Thinking of the usage we do of this function can we rename it? (contentIsHandheld sounds like a function that return a boolean for me)


>-      // Tell the UI to resize the browser controls before calling  updateSize
>+      // Cause a resize of the viewport if the current browser holds a XUL document
>+      let browser = Browser.selectedBrowser;
>+      if (browser.contentDocument instanceof XULDocument)
>+        BrowserView.Util.ensureMozScrolledAreaEvent(browser, w, h);
>+

Maybe we should use the new shiny "contentIsHandheld" function here instead of checking if that a XULDocument?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 438532 [details] [diff] [review]
patch 2

As discuss, r+ with the rename from contentIsHandheld to getViewportMetadata

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed with name change:
http://hg.mozilla.org/mobile-browser/rev/5aecd83c300a

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED on build:
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.4pre) Gecko/20100413 Namoroka/3.6.4pre Fennec/1.1a2pre