656329 - Use a Honeycomb-style action bar on Android tablets

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Description

[Matt Brubeck (:mbrubeck)]

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)]

Attachment: WIP

This works, probably needs a little testing and polishing.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: 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)]

Attachment: 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)]

Attachment: 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?)]

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]]

Attachment: 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)]

Attachment: 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]]

Attachment: 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]

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)]

Attachment: 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]]

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]]

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]]

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]]

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?)]

try using collapse instead of hidden, or just set width/height to zero

Comment 16

[Chris Lord [:cwiiis]]

Attachment: 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)]

Attachment: 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)]

Attachment: part 1 (v2): Remove sidebars and create the action bar

Addresses review comments.  Carrying r=mfinkle.

Comment 19

[Matt Brubeck (:mbrubeck)]

Attachment: part 2 (v2): move tab sidebar to the right

Just adding a checkin comment.

Comment 20

[Matt Brubeck (:mbrubeck)]

Attachment: part 3 (v2): Fix viewport height

Comment 21

[Matt Brubeck (:mbrubeck)]

Comment on attachment 541576 [details] [diff] [review]
part 2.1

Let's move this patch to bug 664149.

Comment 22

[Matt Brubeck (:mbrubeck)]

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)]

Attachment: 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)]

Attachment: part 2 (v4): move tab sidebar to the right

Add missing file.

Comment 25

[Matt Brubeck (:mbrubeck)]

Attachment: 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)]

Attachment: 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)]

TODO: NewTabPopup still needs some changes to work in tablet mode.

Comment 28

[Chris Lord [:cwiiis]]

For whatever reason, part 1 of this patch now breaks input on about pages.

Comment 29

[Chris Lord [:cwiiis]]

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)]

Attachment: 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)]

Comment on attachment 550530 [details] [diff] [review]
part 4: Add a preference

This patch isn't quite complete yet.

Comment 32

[Matt Brubeck (:mbrubeck)]

Attachment: 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)]

Attachment: part 2

Comment 34

[Matt Brubeck (:mbrubeck)]

Attachment: part 3

Comment 35

[Matt Brubeck (:mbrubeck)]

Attachment: 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)]

Some browser-chrome tests need to be changed to work with these patches.

Comment 37

[Matt Brubeck (:mbrubeck)]

(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] (Away Apr 25 - May 5)]

http://hg.mozilla.org/mozilla-central/rev/3e764c25c144

Comment 39

[Aaron Train [:aaronmt]]

Samsung Galaxy Tab 10.1
Mozilla/5.0 (Android, Linux armv7l; rv:9.0a1) Gecko/20110906 Firefox/9.0a1 Fennec/9.0a1