Last Comment Bug 656329 - Use a Honeycomb-style action bar on Android tablets
: Use a Honeycomb-style action bar on Android tablets
Status: VERIFIED FIXED
[inbound]
:
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: All Android
: -- enhancement with 2 votes (vote)
: Firefox 8
Assigned To: Matt Brubeck (:mbrubeck)
:
Mentors:
Depends on: 677670 678108
Blocks: 655762 664149 677669
  Show dependency treegraph
 
Reported: 2011-05-11 09:25 PDT by Matt Brubeck (:mbrubeck)
Modified: 2011-09-14 22:41 PDT (History)
12 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP (10.92 KB, patch)
2011-05-18 14:37 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
WIP 2 (21.54 KB, patch)
2011-05-19 14:22 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
WIP 3 (21.57 KB, patch)
2011-06-14 09:34 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 1 (21.04 KB, patch)
2011-06-14 16:08 PDT, Matt Brubeck (:mbrubeck)
mark.finkle: review+
Details | Diff | Review
part 1.1 (7.60 KB, patch)
2011-06-23 13:59 PDT, Chris Lord [:cwiiis]
no flags Details | Diff | Review
part 2: move tab sidebar to the right (4.64 KB, patch)
2011-06-23 17:08 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 2.1 (7.99 KB, patch)
2011-06-23 19:11 PDT, Chris Lord [:cwiiis]
no flags Details | Diff | Review
part 3: Fix viewport height (5.39 KB, patch)
2011-06-24 16:16 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
Patch 2.05 - Fix XBL binding by using 'collapsed' instead of 'hidden' (942 bytes, patch)
2011-06-27 10:26 PDT, Chris Lord [:cwiiis]
no flags Details | Diff | Review
part 2 (v2): move tab sidebar to the right (4.79 KB, patch)
2011-06-28 12:06 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 1 (v2): Remove sidebars and create the action bar (22.28 KB, patch)
2011-06-28 14:17 PDT, Matt Brubeck (:mbrubeck)
mbrubeck: review+
Details | Diff | Review
part 2 (v2): move tab sidebar to the right (4.86 KB, patch)
2011-06-28 14:18 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 3 (v2): Fix viewport height (5.47 KB, patch)
2011-06-28 14:19 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 2 (v3): move tab sidebar to the right (9.21 KB, patch)
2011-06-30 14:25 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 2 (v4): move tab sidebar to the right (11.86 KB, patch)
2011-06-30 14:33 PDT, Matt Brubeck (:mbrubeck)
mark.finkle: review+
Details | Diff | Review
part 3 (v3): fix viewport height (6.03 KB, patch)
2011-06-30 14:53 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 3 (v4): Fix viewport height (6.05 KB, patch)
2011-06-30 15:24 PDT, Matt Brubeck (:mbrubeck)
mark.finkle: review+
Details | Diff | Review
part 4: Add a preference (5.05 KB, patch)
2011-08-03 15:15 PDT, Matt Brubeck (:mbrubeck)
no flags Details | Diff | Review
part 1 (24.87 KB, patch)
2011-08-04 17:02 PDT, Matt Brubeck (:mbrubeck)
mbrubeck: review+
Details | Diff | Review
part 2 (11.86 KB, patch)
2011-08-04 17:03 PDT, Matt Brubeck (:mbrubeck)
mbrubeck: review+
Details | Diff | Review
part 3 (5.40 KB, patch)
2011-08-04 17:04 PDT, Matt Brubeck (:mbrubeck)
mbrubeck: review+
Details | Diff | Review
part 4: Add a preference (10.75 KB, patch)
2011-08-04 17:09 PDT, Matt Brubeck (:mbrubeck)
mark.finkle: review+
Details | Diff | Review

Description Matt Brubeck (:mbrubeck) 2011-05-11 09:25:22 PDT
As part of the https://wiki.mozilla.org/Fennec/Features/TabletUI project, "tablet mode" in Fennec will use an action bar at the top of the screen, instead of the existing toolbars.

Related Android documentation:
http://developer.android.com/sdk/android-3.0-highlights.html
http://developer.android.com/guide/topics/ui/actionbar.html
Comment 1 Matt Brubeck (:mbrubeck) 2011-05-18 14:37:35 PDT
Created attachment 533428 [details] [diff] [review]
WIP

This works, probably needs a little testing and polishing.
Comment 2 Matt Brubeck (:mbrubeck) 2011-05-19 14:22:14 PDT
Created attachment 533788 [details] [diff] [review]
WIP 2

To do:
* "Home" button.
* Tab list in arrowbox (WIP patch has a dummy/placeholder)
* Need a real icon for the tab button.
Comment 3 Matt Brubeck (:mbrubeck) 2011-06-14 09:34:40 PDT
Created attachment 539233 [details] [diff] [review]
WIP 3

This adds an arrowbox popup for the tab list.  The tabs do not actually appear, however, because of some CSS or XBL issue I haven't figured out yet.  The latest mockups replace this popup with a sidebar, which might be easier to implement anyway...
Comment 4 Matt Brubeck (:mbrubeck) 2011-06-14 16:08:40 PDT
Created attachment 539352 [details] [diff] [review]
part 1

This is a minimal patch to change the urlbar into a fixed "action bar" with buttons for tabs, back/forward, and bookmark.  It removes the right sidebar, but keeps tabs in the left sidebar.

Test build at http://people.mozilla.com/~mbrubeck/fennec-action-bar.apk

TODO (in followup patches):

* Artwork/styling.
* Add a button and popup for the app menu.
* Make the <browser> shorter so it can't scroll behind the action bar.  (This especially affects pages that use touch events.)
* Re-style the tab bar to be a vertical list with thumbnails and titles.
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2011-06-14 21:16:52 PDT
Comment on attachment 539352 [details] [diff] [review]
part 1

* Remove dump in toggleTabSidebar
* Add changes to /core/gingerbread/browser.css as well
Comment 6 Chris Lord [:cwiiis] 2011-06-23 13:59:21 PDT
Created attachment 541491 [details] [diff] [review]
part 1.1

This patch takes a first run at adding a menu-button in tablet mode. Bit disappointed that the arrowbox loses its call-out when you set the position, need to find why...

Also, menu items added by extensions end up with the label reading 'undefined'. Probably a bigger issue than the missing call-out.
Comment 7 Matt Brubeck (:mbrubeck) 2011-06-23 17:08:51 PDT
Created attachment 541552 [details] [diff] [review]
part 2: move tab sidebar to the right

This moves the "tabs" button to the right, and turns the tab sidebar into a floating panel on the right-hand side of the screen.  This patch is a work-in-progress.  It's functional, but needs some cleanup before review.
Comment 8 Chris Lord [:cwiiis] 2011-06-23 19:11:24 PDT
Created attachment 541576 [details] [diff] [review]
part 2.1

Updated menu-adding patch that applies over the top of the part 2 patch and fixes a couple of bugs.
Comment 9 David Spalding 2011-06-24 10:04:25 PDT
Just starting to look at this project. 

Something that I use in FF3/4 a LOT is CTRL+F to find in page. Some users
configure preferences to start search upon user typing. Currently "Find in page"
is a menu item. I would like to see it as a dedicated or optional button
(n00b question: will the Action Bar be customizable as in FF4?), and/or 
a menu choice on a popup menu (if present) on press-and-hold on html page.

I'm not aware of any built-in Honeycomb method for search, so if we're 
shooting for a rigidly Honeycomb-like action this could be out of scope.
Comment 10 Matt Brubeck (:mbrubeck) 2011-06-24 16:16:02 PDT
Created attachment 541854 [details] [diff] [review]
part 3: Fix viewport height

This is now working; it fixes some scrolling and content-size issues.  I think I want to clean it up a bit too before asking for review.
Comment 11 Chris Lord [:cwiiis] 2011-06-26 15:53:56 PDT
Something about the part 2 patch breaks things in browser.js, for me - you get the error "Error: this._chromeTab.updateThumbnail is not a function", and pages appear to either never stop loading or never start. It also breaks session restore.

My guess is that "this._chromeTab = document.getElementById("tabs").addTab();" in browser.js isn't working for some reason, but I don't really know yet, and I don't see how anything in part 2 could break this, but it does (everything is fine before applying that patch).

Maybe there's a reliance on document structure somewhere?
Comment 12 Chris Lord [:cwiiis] 2011-06-26 16:59:21 PDT
Having played a bit more, it seems the thumbnailing breaks if the tab-bar hasn't been visible at some point (so if you remove the initial hide in resizeHandler, everything works).

Is this some kind of lazy-creation in XUL going on? Can we override it?
Comment 13 Chris Lord [:cwiiis] 2011-06-26 17:47:45 PDT
The problem appears to be that XBL isn't bound to hidden elements - Apparently this can be solved by flushing the style before hiding...

Trying to figure out where and what to flush, but I suppose we didn't hit this before because we were moving it out of the way rather than hiding it.
Comment 14 Chris Lord [:cwiiis] 2011-06-26 19:27:39 PDT
For now, I can't think of a trivial way to fix this other than using some other method to hide the tab side-bar, or not using XBL to define tab methods.

The problem is with the tab side-bar being hidden when a tab is created - if that's the case, the XBL won't bind on the created tab, and there's no way I know of to force this without first unhiding its tab-list ancestor.
Comment 15 Mark Finkle (:mfinkle) (use needinfo?) 2011-06-27 07:42:52 PDT
try using collapse instead of hidden, or just set width/height to zero
Comment 16 Chris Lord [:cwiiis] 2011-06-27 10:26:04 PDT
Created attachment 542196 [details] [diff] [review]
Patch 2.05 - Fix XBL binding by using 'collapsed' instead of 'hidden'

Indeed, setting collapsed instead of hidden has the same effect and doesn't break XBL binding. Thanks!
Comment 17 Matt Brubeck (:mbrubeck) 2011-06-28 12:06:19 PDT
Created attachment 542546 [details] [diff] [review]
part 2 (v2): move tab sidebar to the right

This rolls together my original part 2 patch, plus Chris's "collapsed" change and some other fixes.
Comment 18 Matt Brubeck (:mbrubeck) 2011-06-28 14:17:48 PDT
Created attachment 542598 [details] [diff] [review]
part 1 (v2): Remove sidebars and create the action bar

Addresses review comments.  Carrying r=mfinkle.
Comment 19 Matt Brubeck (:mbrubeck) 2011-06-28 14:18:44 PDT
Created attachment 542599 [details] [diff] [review]
part 2 (v2): move tab sidebar to the right

Just adding a checkin comment.
Comment 20 Matt Brubeck (:mbrubeck) 2011-06-28 14:19:16 PDT
Created attachment 542600 [details] [diff] [review]
part 3 (v2): Fix viewport height
Comment 21 Matt Brubeck (:mbrubeck) 2011-06-28 14:20:58 PDT
Comment on attachment 541576 [details] [diff] [review]
part 2.1

Let's move this patch to bug 664149.
Comment 22 Matt Brubeck (:mbrubeck) 2011-06-29 07:30:55 PDT
Comment on attachment 542599 [details] [diff] [review]
part 2 (v2): move tab sidebar to the right

Some TODOs found while testing this patch:

* Tab bar is not treated as a popup (does not dismiss with the back button or tapping elsewhere) - should use pushPopup/popPopup to fix this.
* Tab bar sometimes has the wrong height.
* "New tab opened" notification popup no longer appears.
Comment 23 Matt Brubeck (:mbrubeck) 2011-06-30 14:25:31 PDT
Created attachment 543257 [details] [diff] [review]
part 2 (v3): move tab sidebar to the right

Fix popup behavior and resizing issues, and make sure Gingerbread theme is updated too.
Comment 24 Matt Brubeck (:mbrubeck) 2011-06-30 14:33:55 PDT
Created attachment 543265 [details] [diff] [review]
part 2 (v4): move tab sidebar to the right

Add missing file.
Comment 25 Matt Brubeck (:mbrubeck) 2011-06-30 14:53:07 PDT
Created attachment 543268 [details] [diff] [review]
part 3 (v3): fix viewport height

Rebase on top of part 1+2, and make sure it fixes Gingerbread theme too.

There is an updated test build for Android at http://people.mozilla.com/~mbrubeck/fennec-action-bar-2.apk
Comment 26 Matt Brubeck (:mbrubeck) 2011-06-30 15:24:48 PDT
Created attachment 543281 [details] [diff] [review]
part 3 (v4): Fix viewport height

Fixed a stupid bug in the last patch that broke panning in non-tablet mode.
Comment 27 Matt Brubeck (:mbrubeck) 2011-06-30 18:53:46 PDT
TODO: NewTabPopup still needs some changes to work in tablet mode.
Comment 28 Chris Lord [:cwiiis] 2011-07-27 08:03:41 PDT
For whatever reason, part 1 of this patch now breaks input on about pages.
Comment 29 Chris Lord [:cwiiis] 2011-07-27 08:26:41 PDT
The breakage is due to the change of the this.canCancelPan variable - If you remove the || Utils.IsTablet() from the end of its calculation, it works again. I'm not exactly sure what this variable is used for exactly, so I don't know if this change is necessary or not.
Comment 30 Matt Brubeck (:mbrubeck) 2011-08-03 15:15:46 PDT
Created attachment 550530 [details] [diff] [review]
part 4: Add a preference

This adds a preference "browser.ui.layout.tablet" that can be used to force "tablet mode" on or off.  By default it will be enabled based on the screen size, just as it is now.

This affects only the changes to the main browser UI in this bug; it does not override previous tweaks we've made to large-screen behavior, like the pref styles or the form helper behavior.  The main goal is to be able to check in this code and easily disable it until it is ready to ship.
Comment 31 Matt Brubeck (:mbrubeck) 2011-08-03 15:17:30 PDT
Comment on attachment 550530 [details] [diff] [review]
part 4: Add a preference

This patch isn't quite complete yet.
Comment 32 Matt Brubeck (:mbrubeck) 2011-08-04 17:02:35 PDT
Created attachment 550899 [details] [diff] [review]
part 1

Just rebasing the existing patches, and copying some changes from the default theme to the gingerbread theme that were missed before.
Comment 33 Matt Brubeck (:mbrubeck) 2011-08-04 17:03:06 PDT
Created attachment 550900 [details] [diff] [review]
part 2
Comment 34 Matt Brubeck (:mbrubeck) 2011-08-04 17:04:08 PDT
Created attachment 550902 [details] [diff] [review]
part 3
Comment 35 Matt Brubeck (:mbrubeck) 2011-08-04 17:09:18 PDT
Created attachment 550903 [details] [diff] [review]
part 4: Add a preference

Add a preference "browser.ui.layout.tablet" that can be used to force the tablet layout on (1) or off (0).  When we ship the feature it will default to -1, which will enable tablet mode automatically based on the screen size.  For now it defaults to 0.

My plan is to check in parts 1-4 immediately, then file followup bugs for the remaining issues, visual polish, and any design changes we make.  We can change the pref as we decide to enable this in the various nightly and release channels.
Comment 36 Matt Brubeck (:mbrubeck) 2011-08-04 22:28:15 PDT
Some browser-chrome tests need to be changed to work with these patches.
Comment 37 Matt Brubeck (:mbrubeck) 2011-08-05 16:15:02 PDT
(In reply to comment #36)
> Some browser-chrome tests need to be changed to work with these patches.

Nope, I was wrong.  No regressions found on TryServer.

Pushed to inbound as a single folded patch containing parts 1-4.  This bug will be resolved when the patch lands; I'll file bugs next week for all of the known follow-up work.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e764c25c144
Comment 38 Marco Bonardo [::mak] 2011-08-06 03:02:01 PDT
http://hg.mozilla.org/mozilla-central/rev/3e764c25c144
Comment 39 Aaron Train [:aaronmt] 2011-09-06 07:57:14 PDT
Samsung Galaxy Tab 10.1
Mozilla/5.0 (Android, Linux armv7l; rv:9.0a1) Gecko/20110906 Firefox/9.0a1 Fennec/9.0a1

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