1409262 - Updated openerTabId is not notified via tabs.onUpdated if it is changed by tabs.update()

Status: NEW

(WebExtensions :: General, defect, P5)

Description

[YUKI "Piro" Hiroshi]

The API document https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/update says that addon can change "tas.Tab"'s "openerTabId" by "tabs.update()". However, after I actually set new "openerTabId" for a tab, it is not notified.


Steps to reproduce:

 1. Open two tabs. For example, they have their own ID "1" and "2".
 2. Open debugger for a tab-related addon, like Tab Center Redux.
 3. Add a listener for tabs.onUpdated, like:
    ---------------
    browser.tabs.onUpdated.addListener((aTabId, aChangeInfo, aTab) => {
      console.log('tabs.onUpdated ', aTabId, aChangeInfo, aTab);
    });
    ---------------
 4. Set the ID of one tab as "openerTabId" of another tab, like:
    ---------------
    browser.tabs.update(1, { opwnerTabId: 2 });
    ---------------


Actual result:
Nothing is reported in the console.


Expected:
A message "tabs.onUpdated ..." is reported in the console.


Additional information:
Updated openerTabId is actually applied to the tab. You can confirm that by tabs.get(), like:
---------------
browser.tabs.get(1).then(t=>console.log(t))
---------------

Comment 1

[Bob Silverberg [:bsilverberg]]

Looking at the code for this, I am not surprised that onUpdated is not fired when only the openerId of a tab is updated.

Do we think it should be? Piro, can you confirm if onUpdated fires in Chrome for a similar update?

Comment 2

[YUKI "Piro" Hiroshi]

Hmm, actually Chrome also doesn't fire tabs.onUpdated for openerTabId. Firefox's current behavior is quite compatible to Chrome.

However I still think it should be notified via tabs.onUpdated (both Firefox and Chrome should do that) because some addon needs to know updated openerTabId immediately. For example, an addon "Tree Style Tab"(1) now uses openerTabId to save "parent tab" information for a tab, and another addon "Tab Session Manager"(2) saves/restores tabs with openerTabId. As the result, TST can restore tree of tabs for restored session based on openerTabId. Currently TSM periodically saves session information on every 15 minutes (by default), but because changes of openerTabId is not notified, TSM cannot save updated tree structure immediately.

(1) https://addons.mozilla.org/firefox/addon/tree-style-tab/
(2) https://addons.mozilla.org/firefox/addon/tab-session-manager/

Of course TST can send something message to already registered addons via runtime.sendMessage to notify "openerTabId is updated", but not all addons can do such explicit communication. There are possibly other tab tree addons and other session management addons. Moreover, there possibly other type addons using openerTabId. I think it is reasonable that such addons update itself immediately on tabs.onUpdated for openerTabId.

Comment 3

[Bob Silverberg [:bsilverberg]]

Thanks for doing the research and updating the bug, Piro. I'm going to make this as design-decision-needed so we can decide if we want to support this or not.

Comment 4

[Caitlin Neiman [:caitmuenster]]

Hi Piro, this has been added to the agenda for the October 31, 2017 WebExtensions APIs triage meeting. Would you be able to join us on Vidyo or on IRC? 

Here's a few things about the triage to keep in mind: 

* We normally spend 5 minutes per bug
* The more information in the bug, the better
* The goal of the triage is to give a general thumbs up or thumbs down on a proposal; we won't be going deep into implementation details

Relevant Links: 

* Wiki for the meeting: https://wiki.mozilla.org/Add-ons/Contribute/Triage
* Meeting agenda: https://docs.google.com/document/d/1qqE6fkqr-RNWaFvMpv0Z8O5FLDgQ3AT5eGdbTt7lRGg/edit#
* Vision doc for WebExtensions: https://wiki.mozilla.org/WebExtensions/Vision

Comment 5

[YUKI "Piro" Hiroshi]

OK, I hope to join the meeting on IRC. But it is AM2:30 in Japan, so I possibly miss it. Now I try to write more information for this.

There are three different viewpoints about this bug:

 1) Should openerTabId be updatable or should not?
 2) Which updatable attributes should be notified via tabs.onUpdated? All or only some of them?
 3) Is there any available workaround by addon authors?

1) Actually openedTabId is updatable on Google Chrome. So, to keep compatibility with Google Chrome, it still should be updatable. Otherwise, it becomes more hard to port extensions written for Chrome to Firefox.

2) I think this is the most important point on this bug. The API doc of tabs.onUpdated says that its listener will receive a "changeInfo" object, and changeInfo will have only some attributes:
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/onUpdated#changeInfo
(Actually "isArticle" and some other attributes are also notified - they are undocumented for now.)

On the other hand, tabs.update() receives "updateProperties" and an updateProperties also have only some attributes:
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/update

"tabs.Tab", "changeInfo" and "updateProperties" are similar but different. There are some groups of attributes:

A-1) Updatable / Notifiable (ex. url, pinned)
A-2) Updatable / Unnotifiable (ex. active, openerTabId, autoDiscardable)
B-1) Unupdatable / Notifiable  (ex. status, favIconUrl)
B-2) Unupdatable / Unnotifiable (ex. height, index)

A-1 and B-2 are simple. B-1 seems to be restricted by some reasons. For example title and favIconUrl strongly depend on tab content; that seems reasonable.

And there is a special group A-2, openerTabId is here. I think that attributes in A-2 must have something reason that it must not to be notified via tabs.onUpdated. Actually "active" is notified via tabs.onActivated, because (in my opinion) it is very special and should not handled by generic event tabs.onUpdated. However I couldn't imagine why other attributes "openerTabId" and "autoDiscardable" must not be notified. tabs.Tab also have these attributes, and they don't depend on tab content, so I think it seems reasonable that they are also notified via tabs.onUpdated.

Comment 6

[YUKI "Piro" Hiroshi]

3) This is also important. If there is any workaround to track updated "openerTabId" immediately, there is less reason that "openerTabId" is notified via tabs.onUpdated.

After my experiments, conclusion: it is impossible by addon to know that "openerTabId" is updated by other addon or not.

If it is updated by the addon itself, it can be tracked by self. Actually an example Tree Style Tab addon does it, like:

function attachTabTo(child, parent) {
  ...
  child.apiTab.openerTabId = parent.apiTab.id; // ".apiTab" is a tabs.Tab
  // Cache the last value updated by self.
  // If tabs.Tab's openerTabId is different from this value,
  // that means: openerTabId is changed by any other addon.
  child.apiTab.TSTUpdatedOpenerTabId = child.apiTab.openerTabId;
  browser.tabs.update(child.apiTab.id, { openerTabId: parent.apiTab.id });
  ...
}

However, tabs.Tab's openerTabId is possibly modified by any other addon. If it is updated by someone, it isn't notified via tabs.onUpdated and an addon which uses openerTabId will know the change suddenly on different timing: when the addon tries to get tab information by browser.tabs.get(), when the addon tracks updated attributes of a tab by tabs.onUpdated (the listener receives complete tabs.Tab object as its third argument and it will have updated openerTabId), and others. Because most WebExtensions APIs are asynchronous, the workaround above is not effective to detect whether the openerTabId is updated by self or by others. Before tabs.onUpdated is notified by the above code, any other addon can change the tab's openerTabId andthe listener for tabs.onUpdate will receive changed "status" or something together with the third argument tabs.Tab including modified openerTabId. As the result, the workaround above won't work as expected. The scenario is:

 1) Someone changes the tab's openerTabId to 11 asynchronously.
 2) My addon changes the tab's openerTabId to 10 asynchronously.
    The cached TSTUpdatedOpenerTabId also becomes to 10.
 3) tabs.onUpdated is notified for 1).
    My addon's listener compares TSTUpdatedOpenerTabId(10) with notified openerTabId(11).
    My addon detects that it is changed by others.
    TSTUpdatedOpenerTabId is updated to 11.
 4) tabs.onUpdated is notified for 2).
    My addon's listener compares TSTUpdatedOpenerTabId(11) with notified openerTabId(10).
    My addon wrongly detects that it is changed by others, but actually it was done by self.

Comment 7

[YUKI "Piro" Hiroshi]

Thus, if openerTabId is not notified via tabs.onUpdated, I think that addon authors must give up tracking of dynamically changed openerTabId. instead addon authors always need to scan all tabs and their openerTabId, to know complete relation information of tabs. Such periodical tasks can eat more computing resources. In other words, tabs.onUpdated for openerTabId will reduce such an oxygen thief processes.

Comment 8

[Bob Silverberg [:bsilverberg]]

It was decided that we should diverge from Chrome's behaviour here and notify via onUpdated when openerTabId changes. Approved.

Comment 9

[Morikko]

If FF could notify when openerTabId changes, it would help the compatibility cross extension. Now, when I changed openerTabId value in my extension, tree style tab extension can't know it and thus can't update itself.

Any progress about it ?

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 80 votes.
:robwu, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 11

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 12

[Stefan Richter]

This bug is still relevant. I will try to provide a fix.

Comment 13

[Stefan Richter]

Attachment: Bug 1409262 - Notify when openerTabId changed via tabs.update() r=willdurand

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.