Last Comment Bug 773735 - Do not use tablet layout on devices with screen size "large"
: Do not use tablet layout on devices with screen size "large"
Status: VERIFIED FIXED
: regression
Product: Firefox for Android
Classification: Client Software
Component: Theme and Visual Design (show other bugs)
: 16 Branch
: All Android
: -- normal (vote)
: Firefox 16
Assigned To: Matt Brubeck (:mbrubeck)
:
Mentors:
Depends on:
Blocks: 770079
  Show dependency treegraph
 
Reported: 2012-07-13 11:26 PDT by Matt Brubeck (:mbrubeck)
Modified: 2016-07-29 14:26 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
verified


Attachments
Nightly on 7" Galaxy Tab (298.40 KB, image/png)
2012-07-13 11:26 PDT, Matt Brubeck (:mbrubeck)
no flags Details
patch (49.88 KB, patch)
2012-07-13 12:01 PDT, Matt Brubeck (:mbrubeck)
mark.finkle: review+
mark.finkle: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Matt Brubeck (:mbrubeck) 2012-07-13 11:26:59 PDT
Created attachment 641970 [details]
Nightly on 7" Galaxy Tab

In bug 770079 we switched to using the native tablet layout on all devices with screen size "large" or higher.  This includes 7" tablets, and some 5" devices like some versions of the Galaxy Note.  But our tablet UI, at least as currently implemented does not really work well on these mid-size devices, especially in portrait mode.  (See the attached screenshot.)

I don't think the tablet UI is ready for these devices, and at least for Firefox 15 and 16 it would be better to ship the phone UI for "large" screen sizes.  In a later release we can make the UI adapt more specifically to these devices.

For comparison, the XUL Fennec tablet UI was used only on "xlarge" devices (with an exception for "large" Honeycomb tablets, because the XUL phone UI had some problems on those devices).
Comment 1 Matt Brubeck (:mbrubeck) 2012-07-13 11:34:56 PDT
We can make this change in Native Fennec without re-introducing the complications and duplicate code that bug 770079 removed.

The easiest option is to just rename all of our *-large directories to *-xlarge.  Then we will use the tablet layout only on "xlarge" screens (960x720 "dips" or larger) -- this includes 9" and 10" tablets, and exclude most smaller devices.
Comment 2 Matt Brubeck (:mbrubeck) 2012-07-13 12:01:35 PDT
Created attachment 641986 [details] [diff] [review]
patch

On IRC, madhava wrote:

> mbrubeck: yeah - based on that definition of large, I wouldn't do tablet UI there
> I know that ibarlow had a different layout in mind for 7" that was more an adapted version of the phone UI
> i.e. tabs at the top

This patch just renames every *-large* path to *-xlarge*, and adjusts the isTablet methods.  (It also takes one step toward combining the two isTablet methods into one.)

For anyone following along:  I know the phone layout is not perfect for 7" tablets; we just think it's better than the tablet layout *for now*.  In future releases we will refine the UI for small tablets.
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-13 12:53:08 PDT
Comment on attachment 641986 [details] [diff] [review]
patch

>diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java

>     public static boolean isTablet() {

>+        return GeckoApp.mAppContext.isTablet();

Given the activity issues and static woes, maybe GeckoAppShell should hold the real impl and GeckoApp can delegate to it.

>diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in

> RES_LAYOUT_LARGE = \

> RES_VALUES_LARGE = \

> RES_DRAWABLE_LARGE_MDPI = \

> RES_DRAWABLE_LARGE_HDPI = \

> RES_DRAWABLE_LARGE_XHDPI = \

To minimize confusion, can we rename these variable too? LARGE -> XLARGE

> RESOURCES=$(RES_LAYOUT) $(RES_LAYOUT_LAND_V14) $(RES_LAYOUT_LARGE) $(RES_VALUES) $(RES_VALUES_V11) $(RES_VALUES_LARGE) $(RES_VALUES_LAND_V14) $(RES_VALUES_LARGE_V14) $(RES_XML) $(RES_ANIM) $(RES_DRAWABLE_NODPI) $(RES_DRAWABLE_BASE) $(RES_DRAWABLE_LDPI) $(RES_DRAWABLE_HDPI) $(RES_DRAWABLE_XHDPI) $(RES_DRAWABLE_MDPI_V11) $(RES_DRAWABLE_HDPI_V11) $(RES_DRAWABLE_XHDPI_V11) $(RES_DRAWABLE_LAND_V14) $(RES_DRAWABLE_LAND_MDPI_V14) $(RES_DRAWABLE_LAND_HDPI_V14) $(RES_DRAWABLE_LAND_XHDPI_V14) $(RES_DRAWABLE_LARGE_MDPI) $(RES_DRAWABLE_LARGE_HDPI) $(RES_DRAWABLE_LARGE_XHDPI) $(RES_COLOR) $(RES_MENU) $(RES_RAW)

And here
Comment 5 Matt Brubeck (:mbrubeck) 2012-07-13 14:53:19 PDT
Comment on attachment 641986 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug  	770079

User impact if declined: Some mid-sized devices switch to the large-screen tablet layout.

Testing completed (on m-c, etc.): Landed on inbound 7/13.

Risk to taking this patch (and alternatives if risky): This is mobile-only and low-risk.  All it does is change "large" to "xlarge" in all of our resource directories.  (So it touches a large number number of files, but the only change to each file is a trivial mechanical change to the filename.)

String or UUID changes made by this patch: None.
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-07-13 20:24:40 PDT
https://hg.mozilla.org/mozilla-central/rev/9368a54926a5
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-14 08:53:53 PDT
We need this mobile-only style change to keep some phablets from getting the tablet UI.
Comment 8 Matt Brubeck (:mbrubeck) 2012-07-14 11:17:06 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/102d52e466e6
Comment 9 Aaron Train [:aaronmt] 2012-07-14 13:19:26 PDT
Only able to verify on a Galaxy Tab 2 7" here (1024x600) and I the phone UI.

Verified on Nightly (07/14)

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