Last Comment Bug 770079 - Reload and Bookmark disappear from menu in Nightly on tablet with And. 4.03
: Reload and Bookmark disappear from menu in Nightly on tablet with And. 4.03
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: 16 Branch
: ARM Android
: -- normal (vote)
: Firefox 16
Assigned To: Sriram Ramasubramanian [:sriram]
:
Mentors:
Depends on: 773089 773735
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-01 19:50 PDT by djprius
Modified: 2012-11-01 17:19 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Patch (111.34 KB, patch)
2012-07-08 15:26 PDT, Sriram Ramasubramanian [:sriram]
mark.finkle: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description djprius 2012-07-01 19:50:32 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
Build ID: 20120601045813

Steps to reproduce:

On Acer A100 with ICS 4.03 using Nightly (download of 7/1/12), there is problem with Reload and Bookmarks -- at times.  See below for details


Actual results:

If I start Nightly in portrait view, I now get (this just started) the Reload and Bookmarks as icons next to the URL box (and not on the drop-down menu, as before).  This works fine.  However, if I shift my tablet to landscape view, the icons next to the URL box disappear AND they are NOT in the menu either.

If switch to another app and clear Nightly from the Recent App list and then restart Nightly, the portrait view works correctly again with the icons next to the URL box -- until I shift to landscape view.  The behavior is then repeated as described above.
Comment 1 Sriram Ramasubramanian [:sriram] 2012-07-02 15:53:13 PDT
The series of problems is as follows:
1. Actually LARGE identifer qualifies 7" tablets and up. Which means, we can safely rename XLARGE to LARGE and delete SW600DP folders. 7" tablets will be working fine, and our isTablet() will be something like:

return (getConfiguration().screenLayout & MASK == LARGE);

2. However, Galaxy Note is troubling us. Basically LARGE is for 480 * 640dp devices. Note is identifying itself as a LARGE device! Hence, we cannot kill resources or use the above mechanism (unless we plan to have tablet-ish UI for Note)

3. In case of android tablets, the "status bar" at the bottom is a "software" bar. Hence the screenSize given by Android is not the exact phone's size, but just the screen space for the Activity. Hence a 7" tablet of resolution 1280*600 will just show as 1280*550. This is exactly where by check for "600dp" is getting wrong on 7" tablets.

Here are the options:
1. A Tabletish interface for Note and killing SW600DP.
2. A check for 550dp instead of 600dp for 7" tablets.
Comment 2 djprius 2012-07-03 10:43:31 PDT
My Acer A100 (4.0.3) on Nightly has the following differences:
  a.  If I am holding the tablet in portrait mode on loading Nightly, Reload and Bookmark are icons next to the URL box.

  b.  If I am holding the tablet in landscape mode on loading Nightly, Reload and Bookmark are items on the drop-down Menu list.  Why this difference?

  c. If I open Nightly in landscape mode and then shift to portrait mode, the Reload and Bookmark *remain* in the drop-down Menu (note: this contrasts with (a) above)

  Are these differences in handling Reload and Bookmark deliberate?  Seems to me there should be a consistent handling of Reload and Bookmarks.  (I prefer the icons next to the URL box in all cases -- but I realize some might consider this to be not a good use of screen real estate.)

David
Comment 3 Sriram Ramasubramanian [:sriram] 2012-07-08 15:26:35 PDT
Created attachment 640111 [details] [diff] [review]
Patch

This patch renames *xlarge* to *large* as they support 7"!
(Sigh! I didn't check the resolution mentioned by Android dev guidelines properly. They were in dp! And i was thinking in pixels -- making me thing XLARGE is for 10" tablets.. and LARGE is for phones!)

Note:
layout-large qualifies all 7" tablets and up.
However Note is spoofing itself to be a LARGE device.
If we want a phone-ish UI for Note, we need to rename this to layout-large-v11 - so that only honeycomb+ uses the tablet-ui.
However, when Note gets an ICS updated, we will still show tablet-ish interface for it!
Comment 4 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-09 12:26:25 PDT
Comment on attachment 640111 [details] [diff] [review]
Patch

I want to simplify some of this code, so I like this patch. I am tired of doing work specifically for the Note, when it is lies about it's capabilities anyway, so I am fine with treating it the way it wants to be treated and we can see how things work out.
Comment 5 Sriram Ramasubramanian [:sriram] 2012-07-09 13:33:20 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/727439019fa7
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-07-10 15:50:02 PDT
https://hg.mozilla.org/mozilla-central/rev/727439019fa7
Comment 7 djprius 2012-07-10 18:08:04 PDT
  I am the OP.  Unfortunately, the problem is not corrected (Nightly 7/10/12) on my Acer A100 with 4.0.3.

  If I open Nightly in portrait mode the Reload and Bookmarks are shown as icons next to URL box.  Works fine as long as I do NOT move the tablet to landscape: if I move to landscape, the icons disappear -- and they are NOT in the drop-down menu either.  Even if I move the tablet back to portrait view, the icons are gone and they are not in the menu either.  I need to close and reopen Nightly.

  Hence, one MUST open Nightly in landscape view. This works fine. I can go to portrait view and the reload and bookmarks stay in the menu.

  Not sure what you fixed, but it did not solve my issue.  Sorry.  :)

David
Comment 8 Sriram Ramasubramanian [:sriram] 2012-07-10 19:22:37 PDT
(In reply to djprius from comment #7)
>   I am the OP.  Unfortunately, the problem is not corrected (Nightly
> 7/10/12) on my Acer A100 with 4.0.3.
> 
>   If I open Nightly in portrait mode the Reload and Bookmarks are shown as
> icons next to URL box.  Works fine as long as I do NOT move the tablet to
> landscape: if I move to landscape, the icons disappear -- and they are NOT
> in the drop-down menu either.  Even if I move the tablet back to portrait
> view, the icons are gone and they are not in the menu either.  I need to
> close and reopen Nightly.
> 
>   Hence, one MUST open Nightly in landscape view. This works fine. I can go
> to portrait view and the reload and bookmarks stay in the menu.
> 
>   Not sure what you fixed, but it did not solve my issue.  Sorry.  :)
> 
> David

This has landed in m-c only at 3:50 PST! How can you expect it to be in nightly to be so soon.. to test.. and to reopen it? Wake up tomorrow, update nightly and see if it works. :) And then reopen the bug -- if it is still not fixed!
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-10 19:53:11 PDT
Right, this is not in a Nightly yet. Our process is to make this FIXED, and it will be available in the next Nightly, in this case 7/11/12.
Comment 10 djprius 2012-07-11 08:33:19 PDT
Sorry I jumped the gun.  I assumed, wrongly, that "fixed" meant incorporated in the code.  Next time I'll know better.  :)

I confirm the issue is fixed.  I now have the Reload and Bookmarks icons next to the URL box in both portrait and landscape mode and both work fine.  Thanks for the good work.


David
Comment 11 Sriram Ramasubramanian [:sriram] 2012-07-11 23:28:01 PDT
Comment on attachment 640111 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Tablet support for 7".
User impact if declined: ActionBar will lose the refresh/bookmark icons in URL bar.
Testing completed (on m-c, etc.): Landed in m-c on 07/10
Risk to taking this patch (and alternatives if risky): None. This is the best support for 7" tablets.
String or UUID changes made by this patch: None.
Comment 12 Chris Peterson [:cpeterson] 2012-07-12 09:44:41 PDT
Sriram, if you uplift this patch to Aurora, don't forget to include bug 773089's menu-large-v11 fix for GB Galaxy Note.
Comment 13 Wesley Johnston (:wesj) 2012-07-13 17:49:46 PDT
unbitrotted and pushed:

https://hg.mozilla.org/releases/mozilla-aurora/rev/52890cf794ef
Comment 14 djprius 2012-07-16 07:49:40 PDT
(Is "re-open" correct way to add additional comment after "resolved?)

I am using Acer A100 (4.0.3) on Nightly (7/16/12). 

The fix made several days ago is satisfactory in that it works correctly, but there is a usability issue in the landscape view:  the width of the 'menu' (icon with 3 vertical dots) is too small to reliably touch/activate.  In the portrait mode, it is bit wider (!) and fine.

One would have guessed that if one is wider than the other, it would be landscape view  -- but such is not the case.

Difference is just a millimeter or two, but it makes a big difference in my use.

Note: the right-most icon (here the 'menu') is oftentimes a particular issue because some tablet cases make it hard to touch that spot well.  Hence, please consider making the right-most icon a bit wider than the adjoining one.

David
Comment 15 Sriram Ramasubramanian [:sriram] 2012-11-01 17:19:22 PDT
This has been fixed. Changes have been made to landscape mode to have a better target area for the menu. Please file a new bug in case you want a better menu button in landscape mode.

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