1760294 - browserAction.openPopup doesn't throw even when popup can't be opened

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, defect)

Description

[juraj.masiar]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:99.0) Gecko/20100101 Firefox/99.0

Steps to reproduce:

If popup is not available:

• when the Main Toolbar is hidden
• when Compose window is active (which doesn't have addon toolbar)
  And you call browser.browserAction.openPopup() (for example through the context menu item), the popup won't be opened BUT the operation will not reject the promise.

Actual results:

Returns promise that resolves to undefined (same as for successful opening).

Expected results:

The returned promise should reject because the popup can't be opened.

Comment 1

[John Bieling]

Attachment: Bug 1760294 - Make menus.onClicked a user input event. r=#thunderbird-reviewers

Comment 2

[John Bieling]

This would be a change in behaviour, which could break existing add-ons, which do not catch the error (a rejected Promise throws, if .catch() is not used).

Instead, we could return the id of the tab/window where the popup has been opened (or null, if it failed), but we would divert from Firefox here. Philipp? What do you think?

Comment 3

[juraj.masiar]

If I may, this could also fix existing addons which correctly expects the operation to fail and would correctly fallback (for example in Chrome this operation is expected to fail because by default it's behind flag).
I'm also 100% for staying compatible across browsers API :).
If unsure, maybe just keep the change in Beta channel till the next ESR?

Comment 4

[John Bieling]

for example in Chrome this operation is expected to fail because by default it's behind flag

I cannot seem to find it in the chrome docs, did they remove it?
https://developer.chrome.com/docs/extensions/reference/browserAction/#method-openPopup

Are you able to create a scenario in Firefox, where a call to openPopup() actually fails? Do you know if they throw?
Their documentation does not specify a reject behaviour, did you find a reference?
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/browserAction/openPopup

Comment 5

[juraj.masiar]

Hmmm.... I've found this:
https://github.com/GoogleChrome/developer.chrome.com/issues/204

But I can't find the flag, yet when I run web-ext run -t chromium to test my addon in Chromium, it will work just fine (same as in Firefox), but it fails in Stable - because I can't find the flag.

Regarding Firefox, oh, I think it's broken also there - if you create window of type "popup", which doesn't have toolbar (nor addressbar), then it will - if I see this correctly in my logs - starts opening popup (somewhere) and close it immediately. So not throw, but fail :(.

Comment 6

[John Bieling]

Firefox is using similar code to us:

• FF: https://searchfox.org/mozilla-central/source/browser/components/extensions/parent/ext-browserAction.js#662-665
• TB: https://searchfox.org/comm-central/source/mail/components/extensions/ExtensionToolbarButtons.jsm#730-735

Also in the called function they do not throw. I think Firefox first needs to fix this and then we will move along. Can you file a bug against their implementation, together with a reproducer add-on?

Comment 7

[juraj.masiar]

Oh man, you are right. However...
I'm pragmatic person :)
As this is an extreme corner case in Firefox, I can't imagine this getting anything other than P5 and will never be fixed. What's worse, if someone actually takes time to fix it in Firefox, it would make me even more sad because there is 1000 other much more important things that needs fixing.
And since it's Friday 17:00 here, I'm starting to regret I've even wrote this bug :), I'm sorry, I'm just too tired right now.

Comment 8

[John Bieling]

Let us see what Philipp has to say about this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a76ba32bbcec
Make menus.onClicked a user input event. r=#thunderbird-reviewers,aleca

Comment 10

[Philipp Preißer-Kewisch [:Fallen][☁️📆]]

Let me know if you need any specific info from me, it doesn't look like I can help much as it is?

Comment 11

[John Bieling]

I decided to implement this in Bug 1817803. I do not throw, but return false, if the popup could not be opened.

Closing this as fixed, since I did change the user InputHandler for the menus API