1538378 - regression in 67; tabs.onCreated fires with active set to false for new tabs

Status: VERIFIED FIXED

(WebExtensions :: Frontend, defect)

Description

[Bryan Quigley]

Temporary Containers Version: 0.91
Firefox Version: 67.0b3-1 (snap 193) (also affects 67.0b4-2 just upgraded to today)
Operating System + Version: Ubuntu 19.04 Dev
Non-default Options/Preferences in Temporary Containers: Automatic on.

In Firefox 66:

1. Install https://addons.mozilla.org/en-US/firefox/addon/temporary-containers/
2. Enable Automatic mode in add-on preferences
3. Ctrl - T for new tab, note how you immediately have focus/cursor in the new tab and can type.

Upgrade to Firefox 67 beta
..
3. Ctrl - T for new tab, note how you can't immediately type and have to click on the new tab.

I reported it to the add-on author and they said:
Did some debugging and it turns out that in Firefox Beta 67.0b3 and Firefox Nightly 68.0a1 (2019-03-22) the tabs.onCreated event fires with the active property set to false for Ctrl+T and clicking the new tab icon. TC relies on the correct active value being set, since it hands it down to tabs.create. I'd consider this a serious regression and would advise reporting it to Bugzilla.

• (https://github.com/stoically/temporary-containers/issues/224)

I also had a similar issue previously in https://bugzilla.mozilla.org/show_bug.cgi?id=1489876, but that affected multi-account containers as well. This one so far has only affected temporary containers add-on.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=28bc841f06fc5f4d01defbacc80e017f701a8d57&tochange=5570cf4346a807801c535758f1b82dae1ca63f3f

Triggered by:
5570cf4346a8 Oriol Brufau — Bug 1529411 - Fire onCreated before onRemoved when closing last tab with browser.tabs.closeWindowWithLastTab=false. r=mixedpuppy

Comment 2

[Oriol Brufau [:Oriol]]

OK, so technically the tab is first created and then it's activated immediately after, but exposing this seems more confusing than useful (and not compatible with Chrome). Before bug 1529411, the code awaited a tick and thus the tab was selected at that point. I removed this because a comment provided another reasoning, which did no longer apply.

I have thought about recovering the tick before onCreated, and to avoid reverting bug 1529411, add another tick before onRemoved.
But that's a bit tricky because then onRemoved will happen once the tab is completely destroyed, and this has some problems.

So since bug 1529411 was rather a minor thing for an edge case, I think it's safer to

1. Back out bug 1529411, and uplift.
2. Fix the comment explaining the tick, and add a test.
3. Try to find another approach for bug 1529411.

Comment 3

[Oriol Brufau [:Oriol]]

Attachment: Bug 1538378 - Back out bug 1529411. r?mixedpuppy

Comment 4

[Oriol Brufau [:Oriol]]

Attachment: Bug 1538378 - Test that tabs.onCreated fires with active:true. r?mixedpuppy

Depends on D24818

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/435c308dc978
Back out bug 1529411. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/365ff23dd4c9
Test that tabs.onCreated fires with active:true. r=mixedpuppy

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/435c308dc978
https://hg.mozilla.org/mozilla-central/rev/365ff23dd4c9

Comment 7

[Oriol Brufau [:Oriol]]

Comment on attachment 9053450 [details]
Bug 1538378 - Test that tabs.onCreated fires with active:true. r?mixedpuppy

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1529411
• User impact if declined: tabs.onCreated event will always say that the new tabs are not active, even if they are. This can make webextensions malfunction.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just a back out and a new test.
• String changes made/needed:
• Do you want to request approval of these patches as well?: on

Comment 8

[Pascal Chevrel:pascalc]

Comment on attachment 9053450 [details]
Bug 1538378 - Test that tabs.onCreated fires with active:true. r?mixedpuppy

Backout fix for a 67 regression + added test, approved for 67 beta 6, thanks.

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-beta/rev/338dcd382a65
https://hg.mozilla.org/releases/mozilla-beta/rev/2f5aedeffed9

Comment 11

[Bryan Quigley]

Confirmed fixed in 67.0b6-1. thanks for fixing!

Comment 12

[Victor Carciu]

Verified as fixed also using Win10x64 and macOS 10.13.6 with Firefox 67.0b6 and latest Nightly.