1392176 - Implement optional permission prompts

Status: VERIFIED FIXED

(WebExtensions :: Android, defect, P3)

Comment 1

[Andrew Swan [:aswan]]

Attachment: Bug 1392176 Implement prompts for browser.permissions.request() on Android

Review commit: https://reviewboard.mozilla.org/r/180358/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180358/

Comment 2

[Andrew Swan [:aswan]]

Comment on attachment 8908723 [details]
Bug 1392176 Implement prompts for browser.permissions.request() on Android

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/180358/diff/1-2/

Comment 3

[Andrew Swan [:aswan]]

Kris, you're welcome to comment on the rest if you like but mostly I flagged you to r+ the ext-tabs.js change.

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8908723 [details]
Bug 1392176 Implement prompts for browser.permissions.request() on Android

https://reviewboard.mozilla.org/r/180358/#review186300

::: mobile/android/components/extensions/ext-tabs.js
(Diff revision 2)
> -XPCOMUtils.defineLazyModuleGetter(this, "MatchPattern",
> -                                  "resource://gre/modules/MatchPattern.jsm");

Thank you.

Comment 6

[Andy McKay]

Would you have chance to look Sebastian, hoping to land before 57 merge?

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8908723 [details]
Bug 1392176 Implement prompts for browser.permissions.request() on Android

https://reviewboard.mozilla.org/r/180358/#review187466

LGTM.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38bd8b9bf3d8
Implement prompts for browser.permissions.request() on Android r=kmag,sebastian

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/38bd8b9bf3d8

Comment 10

[Andrew Swan [:aswan]]

Comment on attachment 8908723 [details]
Bug 1392176 Implement prompts for browser.permissions.request() on Android

Approval Request Comment
[Feature/Bug causing the regression]:
none, not a regression

[User impact if declined]:
Users will get install-time prompts for WebExtension permissions, but extensions will be able to add additional permissions at runtime without any prompts.  The install-time and run-time prompts complement each other and the former all landed in Fennec 57 but the run-time prompts in this patch landed shortly after the merge, it would be good to have them all together in the same release.

[Is this code covered by automated tests?]:
no

[Has the fix been verified in Nightly?]:
It has been verified by the author, further verification from QE would be good though.

[Needs manual test from QE? If yes, steps to reproduce]: 
yes, see above

[List of other uplifts needed for the feature/fix]:
none

[Is the change risky?]:
no

[Why is the change risky/not risky?]:
The new code only runs when extensions use the WebExtensions api to request additional permissions at runtime.  Changes to existing code that could introduce regressions are minimal.

[String changes made/needed]:
There are new strings in the patch.  For what its worth, they match existing strings on desktop and the existing desktop translations could be re-used if necessary.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

can we let it ride the train? Thanks
Ni to flod to sign off on the new strings.

Comment 12

[Francesco Lodolo [:flod]]

It's OK to uplift this to beta. 

For the record, there's no such thing as reusing existing desktop translations on Fennec.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8908723 [details]
Bug 1392176 Implement prompts for browser.permissions.request() on Android

This can be a pretty bad UX if a webextension requests new permissions without any validation from the user, taking it 
should be in 57b3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8158695b90e4

Comment 15

[CosminB]

Attachment: OptionalPermissionsPrompt.gif

This issue is verified as fixed on Fennec 58.0a1 (2017-09-25) and Fennec 57.0b3 under Android 7.1.2

The optional permissions are displayed when they are prompted.

Please see the attached video.