Tabs, Separate activity, some ui spec needed

RESOLVED FIXED in Firefox 11

Status

()

Firefox for Android
General
P1
normal
RESOLVED FIXED
6 years ago
10 months ago

People

(Reporter: elan, Assigned: sriram)

Tracking

({feature})

unspecified
Firefox 11
ARM
Android
feature
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
* Separate activity
* some ui spec needed

Updated

6 years ago
Summary: Tabs → Tabs, Separate activity, some ui spec needed
(Assignee)

Comment 1

6 years ago
Created attachment 568668 [details] [diff] [review]
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.
Assignee: nobody → sriram
Attachment #568668 - Flags: review?(mark.finkle)
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
Attachment #568668 - Flags: review?(mark.finkle) → review-
(Assignee)

Comment 3

6 years ago
Created attachment 569136 [details] [diff] [review]
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.
Attachment #568668 - Attachment is obsolete: true
Attachment #569136 - Flags: review?(mark.finkle)
(Assignee)

Comment 4

6 years ago
Created attachment 569139 [details] [diff] [review]
Preserving tabs's title and favicon

This patch preserves the tabs count, title and favicon of the selected tab on the BrowserToolbar on rotation.
Attachment #569139 - Flags: review?(mark.finkle)
Attachment #569139 - Flags: review?(mark.finkle) → review+
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+
Attachment #569136 - Flags: review?(mark.finkle) → review+

Updated

6 years ago
Duplicate of this bug: 695808
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/projects/birch/rev/8db2fba86f75 - fixed the issue.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Depends on: 697095

Updated

6 years ago
Depends on: 697098
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
(Reporter)

Updated

5 years ago
Keywords: feature
Target Milestone: --- → Firefox 11
You need to log in before you can comment on or make changes to this bug.