1881820 - browser.commands.onCommand stops working after the auto-update of addon until browser reload

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P2)

Description

[Max Badryzlov]

Steps to reproduce:

• Open Firefox in a new profile
• Install the last but one nightly build of Sidebery from here: https://github.com/mbnuqw/sidebery/releases/tag/v5.1.1
• Open a new window
• Check that shortcuts works ok: F1(win)/Ctrl+E to open/close sidebar, Alt+Up/Down to select tabs, Alt+Space to activate selection...etc
• In about:addons menu click on the "Check for Updates" option.

Actual results:

After update shortcuts stop working in one of the windows.
Output of Browser Console:

• On update:

20:38:46.946 WebExtension context not found!          ExtensionParent.sys.mjs:1276:13
    getContextById       resource://gre/modules/ExtensionParent.sys.mjs:1276
    recvRemoveListener   resource://gre/modules/ExtensionParent.sys.mjs:1255
    _recv                resource://gre/modules/ConduitsChild.sys.mjs:77
    receiveMessage       resource://gre/modules/ConduitsParent.sys.mjs:469

• On pressing keybindings that should work: No messages
• On pressing "F1(win)/Ctrl+E" keybinding to trigger _execute_sidebar_action:

20:41:28.476 TypeError: can't access property "canAccessWindow", this.extension is null  ext-sidebarAction.js:386:9
  triggerAction  chrome://browser/content/parent/ext-sidebarAction.js:386
  buildKey       resource://gre/modules/ExtensionShortcuts.sys.mjs:463

Expected results:

Shortcuts should work in all windows.

---

mozregression:

Last good revision: ecab7f04e7d616a69e1d213ac8e660ebae99926d
First bad revision: 214490a419fb2f60b4ea03f3bf17c3f0d5bb4d09
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ecab7f04e7d616a69e1d213ac8e660ebae99926d&tochange=214490a419fb2f60b4ea03f3bf17c3f0d5bb4d09

API usage:

browser.commands.onCommand.addListener called on sidebar initialization: https://github.com/mbnuqw/sidebery/blob/192493d11c2980837c1c23a2434f1384e2737931/src/services/keybindings.actions.ts#L1161, https://github.com/mbnuqw/sidebery/blob/192493d11c2980837c1c23a2434f1384e2737931/src/sidebar/sidebar.ts#L144

Repo issues:

https://github.com/mbnuqw/sidebery/issues/1477
https://github.com/mbnuqw/sidebery/issues/1470
https://github.com/mbnuqw/sidebery/issues/1242

Comment 1

[Alex Cornestean]

Hello,

I reproduced the issue on the latest Nightly (125.0a1/20240225092952), Beta (124.0b3/20240223091706) and Release (123.0/20240213221259) under Windows 10 x64.

As per the steps to reproduce, once the extension is updated, the sidebar in the new opened window becomes blank and shortcuts do not work at all. I can click the toolbar button to close/re-open the sidebar, which will populate it with the tabs I have open, however, shortcuts still do not work.

Performing the shortcuts that should work do not log any messages in the browser console, apart from the “F1” keybind that produces the error message mentioned in Comment 0.
The only difference I see on my end is the lack of a warning/error when updating the extension as opposed to what is mentioned in the report.

Comment 2

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

Setting Regressed by field after analyzing regression range found by mozregression in comment #0.

Comment 3

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

Set release status flags based on info from the regressing bug 1835890

:dao, since you are the author of the regressor, bug 1835890, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Bob Hood [:bhood]]

Luca, can you please apply Priority/Severity settings to this report when you get a chance? It will help with release management's triage.

Comment 5

[Dão Gottwald [:dao]]

(In reply to Bob Hood [:bhood] from comment #4)

Luca, can you please apply Priority/Severity settings to this report when you get a chance? It will help with release management's triage.

I'd appreciate that too. I'm planning on setting aside some time to look into this, and would prioritize that based on this bug's categorization.

Comment 6

[Rob Wu [:robwu]]

We shouldn't break shortcut functionality under these circumstances.

Comment 7

[Dão Gottwald [:dao]]

Attachment: Bug 1881820 - Handle SidebarUI not being initialized in updateShortcut. r=rpl

Comment 8

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1881820 - Tweak browser_ext_sidebarAction to cover the fix with a regression test. r?willdurand!

Depends on D205440

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1881820 - Wait for SidebarUI to be initialized before trying to update extension sidebar action shortcut. r?robwu!

Depends on D205753

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb05ea51c885
Handle SidebarUI not being initialized in updateShortcut. r=rpl

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ceb2ab46c14d
Tweak browser_ext_sidebarAction to cover the fix with a regression test. r=willdurand

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/cb05ea51c885
https://hg.mozilla.org/mozilla-central/rev/ceb2ab46c14d

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 9392744 [details]
Bug 1881820 - Handle SidebarUI not being initialized in updateShortcut. r=rpl

Beta/Release Uplift Approval Request

• User impact if declined: see comment 0
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a minimal fix. Luca's patch should make this more robust long-term, but I don't think we need to uplift that.
• String changes made/needed:
• Is Android affected?: No

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 9393350 [details]
Bug 1881820 - Tweak browser_ext_sidebarAction to cover the fix with a regression test. r?willdurand!

Beta/Release Uplift Approval Request

• User impact if declined: n/a (this is the test complementing the fix)
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): this is just a test
• String changes made/needed:
• Is Android affected?: No

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9393350 [details]
Bug 1881820 - Tweak browser_ext_sidebarAction to cover the fix with a regression test. r?willdurand!

Approved for 125.0b6.

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/e1e9fedb0d0a
https://hg.mozilla.org/releases/mozilla-beta/rev/e9fa83dc6173

Comment 17

[Alex Cornestean]

I verified the fix on the latest Nightly (126.0a1/20240328213634) under Windows 10 and Ubuntu 22.04 LTS using the steps from Comment 0 and the shortcuts continue to work in the newly opened window after the extension has been updated. Additionally, no more errors are logged to the browser console.

For comparison I also performed the STR on the latest Beta 125.0b5 (125.0b5/20240327092250), which does not have the fix and the issue is of course still reproducible.

I’ll wait for the release of Beta 125.0b6 to verify the fix there as well, but in the meantime, I will close this issue as Verified-Fixed.

Comment 18

[Alex Cornestean]

Verified as Fixed. Tested on the latest Beta (125.0b6/20240329091918) under Windows 10 and Ubuntu 22.04 LTS using the steps from Comment 0.

The shortcuts continue to work in the newly opened window after the extension has been updated. Additionally, no more errors are logged to the browser console.

Comment 19

[Phabricator Automation]

Comment on attachment 9393353 [details]
Bug 1881820 - Wait for SidebarUI to be initialized before trying to update extension sidebar action shortcut. r?robwu!

Revision D205755 was moved to bug 1889096. Setting attachment 9393353 [details] to obsolete.