577151 - Update TabBrowser API to support groups

Status: RESOLVED DUPLICATE of bug 1907099

(Firefox :: Tabbed Browser, defect)

Description

[Ian Gilman [:iangilman]]

Now that we've settled on a path for Bug 574850, we need to update the API, as we'll be asking our extension authors to change to some degree their extensions. Mardak can follow up with more detail but as a quick list of things we know we need to change:

* Have the standard method of asking for the tabs in the window only return the currently visible tabs for the group 
* Add a new method for iterating over all tabs in all of the groups in the window 
* Add the architecture so that tabs know that they belong to a group 
* Add a unique identifier to each tab

Additional notes: 

* We should give some thought (and survey some add-ons) as to whether changing the standard method to give only the visible tabs is preferable to creating a new method for that. I can see arguments both ways, but I figure it comes down to how the existing API is currently being used (as well as where we want to end up in the future).

* In addition to allowing access to all of this state, we'll probably also need to provide events for when the state changes.

Comment 1

[Ed Lee :Mardak]

The current plan is to update tabbrowser to know about groups, so that each tab knows what group it belongs to and the tabbrowser knows what group of tabs to show.

E.g., tabbrowser.mTabs = [[tab0 group app], [tab1 group foo], [tab2 group bar], [tab3 group foo], [tab4 group baz]]

(In reply to comment #0)
> * Have the standard method of asking for the tabs in the window only return the
> currently visible tabs for the group 
tabbrowser.tabsForGroup(tabbrowser.activeGroup)

Could potentially alias tabbrowser.activeTabs to that, but not necessary.

> * Add a new method for iterating over all tabs in all of the groups in the
> window 
for each (let group in tabbrowser.groups)
  tabbrowser.tabsForGroup(group)

> * Add the architecture so that tabs know that they belong to a group 
tabbrowser-tab.groupId

> * Add a unique identifier to each tab
tabbrowser-tab.tabId

This last thing isn't particularly needed for groups, no? But it'll be needed for the rest of tabcandy and a bunch of add-ons anyway?


Summary:

tabbrowser: {
  get [string] activeGroup 
  set activeGroup([string])

  get [array of strings] groups

  [array of tabbrowser-tabs] tabsForGroup([string])
}

tabbrowser-tab: {
  get [string] groupId
  set groupId([string])

  get [string] tabId
  set tabId([string])
}

What's the expected behavior of closing all the tabs in the active group? Select the next group? (What is "next" as differently-grouped tabs are mixed within tabbrowser child array?) Or don't do anything and assume something handling tab notifications will set the appropriate activeGroup?

Can you set the activeGroup to an id that doesn't already exist? Perhaps to create a new group with new tabs?

What's the ordering of tabbrowser.groups? Whichever groups appear first when iterating down mTabs?

What's the ordering of tabsForGroup? Whatever happens to be the base ordering from mTabs?

Should app tabs have a well-known groupId of "app"?

Comment 2

[Ed Lee :Mardak]

(In reply to comment #1)
> Can you set the activeGroup to an id that doesn't already exist? Perhaps to
> create a new group with new tabs?
Hrm.. This would require tabbrowser users to be in charge of creating group ids. Perhaps there should be an api to just create a new group, and internally we assign a guid similar to what we would do for creating a new tab.

Comment 3

[Ed Lee :Mardak]

In terms of implementation, I was thinking about setting/clearing an active attribute on the active tabs so that css can set the visibility property to hidden/collapsed.

I don't think there's a way to set the activeGroup attribute on the tabbrowser element and have css only show tabs with that matching group.

Various uses of mTabs needs to be updated to use activeTabs, e.g., bookmark open tabs, showing tab list. Session store can still store every tab, but it'll need to track the active group and the group ids on each tab.

addTab should probably inherit the activegroup when adding to end/relative. Not sure if it should take extra hash parameter of tabid and/or groupid. Assuming just inherit current active tab, this would affect all other calls to addTab from say external links, bookmarks, etc.

That seems okay.. except for the app-tab case where tabs opened from an app-tab might want their own group? Oh, and probably shouldn't be able to set active group to the app-tab group as they're always available anyway. ?

Comment 4

[Ed Lee :Mardak]

(In reply to comment #1)
> What's the expected behavior of closing all the tabs in the active group?
Similarly, what should happen by default if all groups have 0 tabs? Right now TabCandy modified tabbrowser.xml to open a new tab if the last tab is closing in bug 576427.

Comment 5

[Philipp von Weitershausen [:philikon]]

The notion of an "active group" is pretty TabCandy-specific. If the general idea is that we're going to weld tabbrowser and TabCandy together, then that's ok I guess. If the idea is to make tabbrowser aware of groups so that other add-ons can make use of it, it seems a bit arbitrary. In Vertical Tabs, for instance, the concept of one active group doesn't exist (though of course Vertical Tabs could simply choose to ignore that bit).

I think it'd be fine if tabbrowser introduced groups solely through an attribute on tabs.  That way the 'tabsForGroup()' method would be a mere wrapper around 'tabs.getElementsByAttribute()'. What to do with empty groups would be up to the user of the tabbrowser API. As far as tabbrowser was concerned, a group would seize to exist when its last tab was removed. The important bit really is that tabbrowser would assign a persistent ID to each tab and manage a group attribute that would be persisted through session store as well.

App tabs could indeed be in a special app tab group, and links opened in a new tab would become part of the same group that the original link was in, except perhaps for app tabs.

Comment 6

[Ed Lee :Mardak]

(In reply to comment #5)
> The notion of an "active group" is pretty TabCandy-specific.
I guess a better name would be .selectedGroup just like .selectedTab. And maybe .selectedTabs.

I wonder how this will interact with the multiple-selected-tabs implementation.

Comment 7

[Aza Raskin [:aza]]

> What's the expected behavior of closing all the tabs in the active group?
> Select the next group? (What is "next" as differently-grouped tabs are mixed
> within tabbrowser child array?) Or don't do anything and assume something
> handling tab notifications will set the appropriate activeGroup?

Because there is probably no association between the spatial ordering of the groups and the linear ordering in the tabbrowser object, it makes sense to not do anything and assume something handling tab notifications will set the appropriate activeGroup.

> 
> Can you set the activeGroup to an id that doesn't already exist? Perhaps to
> create a new group with new tabs?

I'd say no. Creating a new group seems like a strange side-effect to an undefined action. I'd have it just throw an error and add another function for creating a new group?

> 
> What's the ordering of tabbrowser.groups? Whichever groups appear first when
> iterating down mTabs?
> 

Can they be kept in chronological order of being created? Like how if you keep .push()ing onto an array the things pushed later will come further down the array. That way linear views of groups (tree-style tabs) will have the groups reported in the right order.


> What's the ordering of tabsForGroup? Whatever happens to be the base ordering
> from mTabs?

Yes?

> 
> Should app tabs have a well-known groupId of "app"?

That makes sense.

Comment 8

[Aza Raskin [:aza]]

> That seems okay.. except for the app-tab case where tabs opened from an app-tab
> might want their own group? Oh, and probably shouldn't be able to set active
> group to the app-tab group as they're always available anyway. ?

Zooming into the app-tab group seems strange indeed. I need to do the spec. See bug #578553

Comment 9

[B.J. Herbison]

Is this the same as the 2017 bug 1357214 which was marked WONTFIX?

Comment 10

[Stephen Thompson [:sthompson]]

Marking this as a duplicate. Firefox 139 will include Tab Groups on by default. Tabbrowser is fully aware of tab groups and the work was tracked against bug 1907099