896555 - Remember permissions for an entire session

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect)

Description

[Marco Castelluccio [:marco]]

Attachment: remember_session_android

On b2g, when an user accepts a permission prompt, the permission is granted for an entire session.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 779271 [details] [diff] [review]
remember_session_android

LGTM, but adding Wes in case he can think of any other side effects. Keep in mind, this prompt handles webapps and the Firefox browser.

Comment 2

[Wesley Johnston (:wesj)]

Comment on attachment 779271 [details] [diff] [review]
remember_session_android

Review of attachment 779271 [details] [diff] [review]:
-----------------------------------------------------------------

AFAICT, this is fine.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/decc061fbd43

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Backed out for causing robocop-2 failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b36617ed67c

https://tbpl.mozilla.org/php/getParsedLog.php?id=25679439&tree=Mozilla-Inbound

Comment 5

[Marco Castelluccio [:marco]]

(In reply to Mark Finkle (:mfinkle) from comment #1)
> Keep in mind, this prompt handles webapps and the Firefox browser.

So maybe for this and for bug 832960 we should check if the request comes from an application?

Comment 6

[Marco Castelluccio [:marco]]

Attachment: remember_session_android v2

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 782813 [details] [diff] [review]
remember_session_android v2

>diff --git a/mobile/android/components/ContentPermissionPrompt.js b/mobile/android/components/ContentPermissionPrompt.js

>+        } else if(isApp || entityName == "desktopNotification") {

nit: add a space after the 'if'

Make a Try run just to be safe

Comment 8

[Marco Castelluccio [:marco]]

Attachment: remember_session_android v3

Already did a try run together with the patch in bug 832960.
Carrying r+.

Please land 832960 and this one together (this applies on top of that patch).

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/34a46f10c5a0

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34a46f10c5a0