Checking "Remember this decision" for WebRTC prompts in PBM should store for session only

VERIFIED FIXED in Firefox 53

Status

()

Firefox
Device Permissions
P1
normal
VERIFIED FIXED
5 months ago
5 months ago

People

(Reporter: johannh, Assigned: johannh)

Tracking

Trunk
Firefox 55
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox53+ verified, firefox54+ verified, firefox55 verified)

Details

(Whiteboard: [fxprivacy])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Right now I can persist this setting permanently, even if set in private mode. We should probably get it in line with the other prompts.

[Tracking Requested - why for this release]:
Violating the PBM promise of not storing user data except bookmarks etc.
Flags: qe-verify+
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 2

5 months ago
mozreview-review
Comment on attachment 8849490 [details]
Bug 1348257 - Hide the "always remember" checkbox for webrtc prompts in PBM.

https://reviewboard.mozilla.org/r/122276/#review124724

LGTM, but I think we should have a test for this.

I wonder if we can prevent this sort of bug by having a PBM check at a lower level (like in SitePermissions.jsm maybe).

Also I wish MozReview would highlight matching braces, haha.
Attachment #8849490 - Flags: review?(nhnt11)
Yeah, I thought I'd try if I can sneak this in without a test :D

Technically I'm not even sure if this needs one. Since we test the API behavior for correctness there's not much that can go wrong assuming nobody outright deletes the option.

Considering this, would you be ok with adding a test in a new bug (for all permission prompts, not just WebRTC)? I'd like to uplift this ASAP and there's bug 1349513 which would have to be resolved first to avoid littering b/b/c/test/general.
Comment hidden (mozreview-request)

Comment 5

5 months ago
mozreview-review
Comment on attachment 8849490 [details]
Bug 1348257 - Hide the "always remember" checkbox for webrtc prompts in PBM.

https://reviewboard.mozilla.org/r/122276/#review124890
Attachment #8849490 - Flags: review?(nhnt11) → review+
Comment hidden (mozreview-request)

Comment 7

5 months ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0837622d7105
Hide the "always remember" checkbox for webrtc prompts in PBM. r=Nihanth

Comment 8

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0837622d7105
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment on attachment 8849490 [details]
Bug 1348257 - Hide the "always remember" checkbox for webrtc prompts in PBM.

Approval Request Comment
[Feature/Bug causing the regression]: Was originally implemented in bug 1282768
[User impact if declined]: Users may accidentally persist the fact that they've visited a website if they permanently deny a camera prompt in private mode.
[Is this code covered by automated tests?]: Kind of. The API for the checkbox is tested, but hiding it in PBM will receive tests once bug 1349513 is solved.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: Go to https://permission.site in private mode. Click on "notifications". Check if you can persist your choice permanently.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Very simple frontend change that is trivial to manually verify.
[String changes made/needed]: None
Attachment #8849490 - Flags: approval-mozilla-beta?
Attachment #8849490 - Flags: approval-mozilla-aurora?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(brindusa.tot)
tracking-firefox53: ? → +
tracking-firefox54: --- → +
I can confirm that in PBM the checkbox to "Remember this decision" for WebRTC prompts is not available anymore and the permissions can be given only temporarily for: Camera, Microphone, Camera and Microphone, Notifications and Screen sharing. Tested using the latest Nightly 55.0a1 (Build ID: 20170326030204) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12 on the following websites: 
https://permission.site/
https://people-mozilla.org/~fqueze2/webrtc/

I do have a question related to the way the temporary notifications are given. For example, if I give a temporary permission for the camera and I refresh the page, then the permission is requested again. But, if I give a temporary permission for the notifications and I refresh the page, the permission is still granted. 
Johann, is this behavior intended?
Flags: needinfo?(jhofmann)
Flags: needinfo?(brindusa.tot)
Yes, the allow button for notifications is intended to permanently allow, see bug 1341742. :)
Flags: needinfo?(jhofmann)
Comment on attachment 8849490 [details]
Bug 1348257 - Hide the "always remember" checkbox for webrtc prompts in PBM.

Verified fix for a privacy issue, seems very minor/low risk, let's take it on beta.
Attachment #8849490 - Flags: approval-mozilla-beta?
Attachment #8849490 - Flags: approval-mozilla-beta+
Attachment #8849490 - Flags: approval-mozilla-aurora?
Attachment #8849490 - Flags: approval-mozilla-aurora+
Needs rebasing for Aurora uplift.
Flags: needinfo?(jhofmann)
nvm, was a trivial rebase
Flags: needinfo?(jhofmann)

Comment 16

5 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/93a70cd28ef9
status-firefox54: affected → fixed

Comment 17

5 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/5d3641819ecc
status-firefox53: affected → fixed
(In reply to Johann Hofmann [:johannh] from comment #12)
> Yes, the allow button for notifications is intended to permanently allow,
> see bug 1341742. :)

Thanks Johann, based on this and on Comment 11, setting the tracking flag for firefox55 to verified.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified

Comment 19

5 months ago
Verified as fixed on Firefox Aurora 54.0a2 and on Firefox Beta 53.0b8 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.
I can confirm that in PBM the "Remember this decision" checkbox for WebRTC prompts isn't available anymore and the permissions can be given only temporarily for: Camera, Microphone, Camera and Microphone, Location and Screen/Window sharing.

However, in PBM the "Remember this decision" checkbox for Notifications is still available due to bug 1341742 which hasn't been uplifted.
Shouldn't this be uplifted?
status-firefox53: fixed → verified
status-firefox54: fixed → verified
Flags: needinfo?(jhofmann)
Flags: needinfo?(elancaster)
(In reply to Hani Yacoub from comment #19)
> Verified as fixed on Firefox Aurora 54.0a2 and on Firefox Beta 53.0b8 on
> Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.
> I can confirm that in PBM the "Remember this decision" checkbox for WebRTC
> prompts isn't available anymore and the permissions can be given only
> temporarily for: Camera, Microphone, Camera and Microphone, Location and
> Screen/Window sharing.
> 
> However, in PBM the "Remember this decision" checkbox for Notifications is
> still available due to bug 1341742 which hasn't been uplifted.
> Shouldn't this be uplifted?

No, the patch for bug 1341742 contains strings and can't be uplifted. The checkbox there is fine, it will only save for session duration in PBM.
Flags: needinfo?(jhofmann)
Flags: needinfo?(elancaster)
You need to log in before you can comment on or make changes to this bug.