695152 - Tabs, Separate activity, some ui spec needed

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Erin Lancaster [:elan]]

* Separate activity
* some ui spec needed

Comment 1

[Sriram Ramasubramanian [:sriram]]

Attachment: Initial Patch

This patch moves the tabs to a popupwindow in same activity.
Since "Tabs" is held separate, the popup shows even when Gecko is loading.
The tabs's title, url, favicon (and even new tabs opening) are updated in the popup when it is showing (for such events happening in the background).
Close tabs work by switching to previous tab in the list.

Some followups:
1. There are 2 data structures used: one for fast retrieval, the other for preserving the order. This needs to be optimized (ArrayList<Tab> is same as ArrayList<Integer> though).
2. The close tabs does something with gecko when the tab closed is the selected tab. I'm failing to understand the reason. The java side sends proper messages.
3. The tab popup has a height of 50% on portrait mode. This need to be changed with the list's height. I would like to take this as a followup bug with the actual styling for the popupwindow.

Comment 2

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

Comment on attachment 568668 [details] [diff] [review]
Initial Patch

This patch is pretty much ready to land except:
* Move the "New Tab" string into a resource (don't use "+ add tab" as the text either)
* I want to use a convention for our JS->Java messages, so for the tab stuff:
** "onAddTab" -> "Tab:Added"
** "onCloseTab" -> "Tab:Closed"
** "onSelectTab" -> "Tab:Selected"

While we are doing this, change the Java->JS names:
* "tab-add" -> "Tab:Add"
* "tab-load" -> "Tab:Load"
* "tab-select" -> "Tab:Select"
* "tab-close" -> "Tab:Close"

I think we'll be r+ ready with those changes

Comment 3

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch for tabs tray

Made the required changes.
This also fixes the "close-tab" problem.
  - When selected tab is closed, a tab is explicitly selected _before_ closing the tab.
  - When a background tab is closed, a tab is explicitly selected _after_ closing the tab.

Comment 4

[Sriram Ramasubramanian [:sriram]]

Attachment: Preserving tabs's title and favicon

This patch preserves the tabs count, title and favicon of the selected tab on the BrowserToolbar on rotation.

Comment 5

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

Comment on attachment 569136 [details] [diff] [review]
Patch for tabs tray

Just some questions:
* Do we need to keep Tabs.java order array? or can we try to remove it in the future?
* the screen shot at http://cl.ly/0w3I311s2a3w3g0r2B3C shows the "tabs" button is a little offset vertically from the rest of the URL bar. We can file a followup for fixing that, but I wanted to point it out.
* File a followup to fix the "prompt" message handler


r+

Comment 7

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/projects/birch/rev/8db2fba86f75 - fixed the issue.