Closed Bug 1710359 Opened 5 months ago Closed 3 months ago

Experimental API Add-on requires new permission (faulty unspecific warning)

Categories

(Thunderbird :: Add-Ons: General, defect)

x86_64
Windows 10
defect

Tracking

(thunderbird_esr78? fixed, thunderbird88 wontfix, thunderbird89 wontfix, thunderbird90 wontfix, thunderbird91? fixed)

RESOLVED FIXED
91 Branch
Tracking Status
thunderbird_esr78 ? fixed
thunderbird88 --- wontfix
thunderbird89 --- wontfix
thunderbird90 --- wontfix
thunderbird91 ? fixed

People

(Reporter: axel.grude, Assigned: TbSync)

References

Details

Attachments

(2 files, 1 obsolete file)

I got the following bug report from a user of my Add-on QuickFolders:

I just noticed a "warning" icon on the Thunderbird hamburger menu.
When I click on it, the message is: Quickfolders requires new permissions

I don't see anything that seems related to addressing the notice.

the warning is nonsense because QF needs full permissions as it is still a "mail extension with experimental API" - we are rewriting the Add-on but it is going to take many months or longer.

the only permission that was added to manifes.json few weeks ago:

"permissions": ["notifications"],

There should be no warnings until it is fully converted, and then the complete list of required permissions should be displayed. At the moment the user will only be guessing - he/she doesn't even see what "new permission" was added, which is not a good experience.

We exclude all permissions the add-on is requesting from the permission window, if it uses Experiments. I think it would be confusing to see individual permissions alongside the full access permission.

To get this right, we should suppress those permissions update popups, if Experiments are present. But should we fire a permission update popup, once the Experiment is removed, so the user can see the individual permissions, or can we assume those are implicitly granted and not do anything?

Or should we always show all permissions and therefore keep showing permission update popups, regardless of Experiments being used or not?

(In reply to John Bieling (:TbSync) from comment #1)

We exclude all permissions the add-on is requesting from the permission window, if it uses Experiments. I think it would be confusing to see individual permissions alongside the full access permission.

To get this right, we should suppress those permissions update popups, if Experiments are present. But should we fire a permission update popup, once the Experiment is removed, so the user can see the individual permissions, or can we assume those are implicitly granted and not do anything?

I think once the Add-on isn't experimental anymore the user will still have to be able to grant informed consent to whatever permissions the Add-on needs in the future, so yes, it should ask for the explicit permissions once. btw, can the permissions be looked up / changed afterwards?

btw, can the permissions be looked up / changed afterwards?

Yes, in the add-on management page, there is a permission tab. But you cannot change them. You cannot disallow a requested permission, you can only reject to install the add-on.

Assignee: nobody → john
Status: NEW → ASSIGNED
Attachment #9229556 - Attachment description: Bug 1710359 - Supress permission update prompts, if add-on already has full access permission. r=mkmelin → Bug 1710359 - Suppress permission update prompts, if add-on already has full access permission. r=mkmelin

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/7437d26d24e0
Suppress permission update prompts, if add-on already has full access permission. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch

Sorry John, the tests failed horribly, so I backed this out:
https://hg.mozilla.org/comm-central/rev/15f7de36ccc2b1de782d40e05991c7eb751dceaf

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

For whatever strange reason, moz-phab added the files empty (0 Byte). Worked on second attempt. Try run:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=047df7f86bc6643b3ec00fa90acbdaac7e85484d

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/741583afa67f
Suppress permission update prompts, if add-on already has full access permission. r=mkmelin

Status: REOPENED → RESOLVED
Closed: 3 months ago3 months ago
Resolution: --- → FIXED

Comment on attachment 9229556 [details]
Bug 1710359 - Suppress permission update prompts, if add-on already has full access permission. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Add-on authors start to migrate from full experiments towards WebExtensions, which involves asking for new permissions. However, Experiments already have "full" permissions and asking for additional ones will confuse users and they might even decline the new permission, which is jeopardizing the migration effort. This has landed in TB91, but TB78 will be around for a while and I would like to prevent another add-on version split (one version for TB78 and one version for TB91) and therefore request uplift, even though this did not go through the full beta cycle.

Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Low.

Attachment #9229556 - Flags: approval-comm-esr78?
Attached patch bug1710359_esr78.patch (obsolete) — Splinter Review
Attachment #9229556 - Flags: approval-comm-esr78?

Comment on attachment 9229663 [details] [diff] [review]
bug1710359_esr78.patch

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Add-on authors start to migrate from full experiments towards WebExtensions, which involves asking for new permissions. However, Experiments already have "full" permissions and asking for additional ones will confuse users and they might even decline the new permission, which is jeopardizing the migration effort. This has landed in TB91, but TB78 will be around for a while and I would like to prevent another add-on version split (one version for TB78 and one version for TB91) and therefore request uplift, even though this did not go through the full beta cycle.

Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Low.

Attachment #9229663 - Flags: approval-comm-esr78?
See Also: → 1715968

Comment on attachment 9229663 [details] [diff] [review]
bug1710359_esr78.patch

[Triage Comment]
Approved for 78.12.0

Attachment #9229663 - Flags: approval-comm-esr78? → approval-comm-esr78+

Patch was missing the XPI files.
I also added a small change and now check if extension object exists, before using it.

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):

Attachment #9229663 - Attachment is obsolete: true
Attachment #9230244 - Flags: approval-comm-esr78?

Comment on attachment 9230244 [details] [diff] [review]
bug1710359_esr78_v2.patch

[Triage Comment]
Approved 78.12.0

Attachment #9230244 - Flags: approval-comm-esr78? → approval-comm-esr78+
You need to log in before you can comment on or make changes to this bug.