Closed Bug 1463751 Opened 6 years ago Closed 6 years ago

Tab-specific data not updated when tab is moved to another window and old window closes


(WebExtensions :: Frontend, defect, P1)



(firefox62 verified)

Tracking Status
firefox62 --- verified


(Reporter: Oriol, Assigned: Oriol)




(2 files)

The fix in bug 1451176 assumed that the "tab-detached" event would be emitted before "tab-select". This is not the case when the tab that is moved between windows is the last one of the old window, and thus the window closes.

 1. Install in about:debugging
 2. You need at least two open windows, one of which with a single tab
 3. Click the browser action in that window, it will display a tab-specific badge.
 4. Drag the tab into another window, the old window will close.

Result: the badge is not displayed. But if you select another tab, and go back to the former one, then the button will be updated and the badge will be displayed.

It seems this is because "tab-detached" events can be emitted by two different codes:

In the second one, it's too late.
My first attempt was with "tab-attached". It seems to be emitted before "tab-select", but "tab-attached" is delayed a tick in order to have the tab index. This was probably a race condition, and my test failed due to this delay (the values were checked before being updated).

So now I'm trying adding a new "tab-adopted" event. Seems to work, will post a patch soon.
Assignee: nobody → oriol-bugzilla
Comment on attachment 8980895 [details]
Bug 1463751 - Tab-specific data not updated when tab is moved to another window and old window closes

Just a few nits below, primarily cleanup some comments and code documentation.

::: browser/components/extensions/parent/ext-browser.js:191
(Diff revision 2)
>      // fromBrowse will be false in case of e.g. a hash change or history.pushState
>      let fromBrowse = !(flags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
>      this.emit("location-change", tab, fromBrowse);
>    }
> -  tabDetached(eventType, {nativeTab, adoptedBy}) {
> +  tabAdopted(eventType, {nativeTab, adoptedTab}) {

Doc string on this function.  Seeing the changes below, I'd like to change the args to something like {adoptingTab, adoptedTab} or {newTab, oldTab}

::: browser/components/extensions/parent/ext-browser.js:312
(Diff revision 2)
> +  adopt(nativeTab, adoptedTab) {
> +    if (!this.adoptedTabs.has(adoptedTab)) {
> +      this.adoptedTabs.set(adoptedTab, nativeTab);
> +      this.setId(nativeTab, this.getId(adoptedTab));
> +      this.emitAdopted(nativeTab, adoptedTab);

Seems the only place this is called, just call this.emit here.

::: browser/components/extensions/parent/ext-browser.js:376
(Diff revision 2)
>          let {adoptedTab} = event.detail;
>          if (adoptedTab) {
> -          this.adoptedTabs.set(adoptedTab,;
> -
>            // This tab is being created to adopt a tab from a different window.
>            // Copy the ID from the old tab to the new.

this comment no longer makes sense, ditto with comments in other areas where adopt is now called.
Attachment #8980895 - Flags: review?(mixedpuppy) → review+
Keywords: checkin-needed
Pushed by
Tab-specific data not updated when tab is moved to another window and old window closes r=mixedpuppy
Keywords: checkin-needed
Priority: -- → P1
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Attached image tab specific data.gif
Reproduced in Firefox 62.0a1 (20180523100147).
Retested and verified in Firefox 62.0a1 (20180605100102) on Windows 10 64Bit and MacOS 10.13.4.
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.