Closed Bug 896555 Opened 6 years ago Closed 6 years ago

Remember permissions for an entire session

Categories

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

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 25

People

(Reporter: marco, Assigned: marco)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch remember_session_android (obsolete) — Splinter Review
On b2g, when an user accepts a permission prompt, the permission is granted for an entire session.
Attachment #779271 - Flags: review?
Attachment #779271 - Flags: review? → review?(mark.finkle)
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.
Attachment #779271 - Flags: review?(wjohnston)
Attachment #779271 - Flags: review?(mark.finkle)
Attachment #779271 - Flags: review+
Comment on attachment 779271 [details] [diff] [review]
remember_session_android

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

AFAICT, this is fine.
Attachment #779271 - Flags: review?(wjohnston) → review+
Keywords: checkin-needed
(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?
Attached patch remember_session_android v2 (obsolete) — Splinter Review
Assignee: nobody → mcastelluccio
Attachment #779271 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #782813 - Flags: review?(mark.finkle)
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
Attachment #782813 - Flags: review?(mark.finkle) → review+
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).
Attachment #782813 - Attachment is obsolete: true
Attachment #783200 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/34a46f10c5a0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.