Experimental API Add-on requires new permission (faulty unspecific warning)
Categories
(Thunderbird :: Add-Ons: General, defect)
Tracking
(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)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
18.57 KB,
patch
|
rjl
:
approval-comm-esr78+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•4 years ago
|
||
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?
Reporter | ||
Comment 2•4 years ago
•
|
||
(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?
Assignee | ||
Comment 3•4 years ago
|
||
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 | ||
Comment 4•4 years ago
|
||
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
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
Updated•4 years ago
|
Comment 6•4 years ago
|
||
Sorry John, the tests failed horribly, so I backed this out:
https://hg.mozilla.org/comm-central/rev/15f7de36ccc2b1de782d40e05991c7eb751dceaf
Assignee | ||
Comment 7•4 years ago
|
||
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
Assignee | ||
Comment 9•4 years ago
|
||
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.
Assignee | ||
Comment 10•4 years ago
|
||
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
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.
Comment 12•4 years ago
|
||
Comment on attachment 9229663 [details] [diff] [review]
bug1710359_esr78.patch
[Triage Comment]
Approved for 78.12.0
Comment 13•4 years ago
|
||
bugherder uplift |
Thunderbird 78.12.0:
https://hg.mozilla.org/releases/comm-esr78/rev/771c51fc826b
Assignee | ||
Comment 14•4 years ago
|
||
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):
Comment 15•4 years ago
|
||
Backout esr78 (771c51fc826b)
https://hg.mozilla.org/releases/comm-esr78/rev/a5fa5ecaf760c673ff1a54aa2dad69f29a932f90
Comment 16•4 years ago
|
||
bugherder uplift |
Thunderbird 78.12.0:
https://hg.mozilla.org/releases/comm-esr78/rev/3f33496fd2d4
Updated•4 years ago
|
Comment 17•4 years ago
|
||
Comment on attachment 9230244 [details] [diff] [review]
bug1710359_esr78_v2.patch
[Triage Comment]
Approved 78.12.0
Comment 18•4 years ago
|
||
bugherder uplift |
Thunderbird 78.12.0
https://hg.mozilla.org/releases/comm-esr78/rev/3f33496fd2d4
Description
•