Closed Bug 1601256 Opened 4 years ago Closed 4 years ago

privateBrowsingId is added to the link in Settings - Autoplay in Preferences

Categories

(Firefox :: Site Permissions, defect, P3)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
Firefox 73
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- unaffected
firefox72 --- verified
firefox73 --- verified
firefox74 --- verified

People

(Reporter: Gabi, Assigned: pbz)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached image blockprivatemode.png
  • [Affected versions]:
    Beta 72.0b2
    Nightly 73.0a1

  • [Affected platforms]:
    macOS 10.15
    Windows 10 x64
    Windows 8.1 x64
    Ubuntu 14.4

  • [Steps to reproduce]:

  1. Launch Firefox
  2. Open a new private window
  3. Open https://edition.cnn.com/videos
  4. Open the Site information panel
  5. Change block autoplay from Block Audio to any other block autoplay option
  6. Go to about:preferences#privacy and check the Permissions section in normal browsing mode
  7. Check the website in Settings - Autoplay
  • [Expected result]:
    Website link is added in Autoplay - Settings panel with the autplay option added in the private window

  • [Actual result]:
    Websiste link is added in Autoplay - Settings preferences with privateBrowsingId=

[Regression range]:
Will add regression asap.

Has Regression Range: --- → no
Has STR: --- → yes

Probably from the changes that Paul made. We should probably avoid showing these permissions and clean them up on exit. Paul, can you take a look whenever you have time?

Flags: needinfo?(pbz)
Component: Preferences → Site Permissions

Thanks Johann! Yes, that's caused by Bug 1422056.

Do we want to hide private browsing permissions in this and the other permission lists in general? What exactly do you mean by cleaning them up on exit?

Assignee: nobody → pbz
Status: NEW → ASSIGNED
Flags: needinfo?(pbz) → needinfo?(jhofmann)
Regressed by: 1422056

Thinking more about this, maybe instead of hiding the entries, we should update the permission list UI and add an indicator for private browsing entries. That's the proposal in Bug 1589608. That would also fix the other permission lists in about:preferences.

While that works for containers, it seems a bit creepy for private pages, no? i.e. if you close a private window the permissions aren't automatically gone, so we would keep showing them there.

I think we should hide them and remove the permissions when doing sanitize-on-shutdown e.g. like this https://searchfox.org/mozilla-central/rev/8bc24752246aeac8a9aed566cf1caccf88d97d11/browser/modules/Sanitizer.jsm#180

Flags: needinfo?(jhofmann)

Or, even simpler, we change page info code to set the permissions to session lifetime if in private mode.

OR (sorry) we update permission manager to automatically hand out session expiry to permissions with pbId attributes.

(In reply to Johann Hofmann [:johannh] from comment #6)

OR (sorry) we update permission manager to automatically hand out session expiry to permissions with pbId attributes.

Yes, this seems like the way to go, I think.

However this on its own won't solve the issue in comment 0.

Sorry, I didn't realize we don't really have persistent PB permissions.

(In reply to Johann Hofmann [:johannh] from comment #6)

OR (sorry) we update permission manager to automatically hand out session expiry to permissions with pbId attributes.

That's a good idea.

So I'll work on two patches, one for the session expiry perms for PB and one for hiding the PB permission entries in the list.
I've also thought about hiding session expiry permissions in settings in general. But then we would loose the ability to clear in-session.

Priority: -- → P3

Reproduced on latest Nightly 73.0a1 (10.12.2019) on Windows 7.

Pushed by pzuhlcke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/82a7408c8b32
nsPermissionManager: Store all private browsing permissions with session expiry. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/adec5f2eb156
Hide private browsing session permissions in preferences permission lists. r=johannh

Thanks Paul. Please remember to nominate this for uplift after a few days of baking time. :-)

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 73

(In reply to :ehsan akhgari from comment #13)

Thanks Paul. Please remember to nominate this for uplift after a few days of baking time. :-)

Flags: needinfo?(pbz)

Comment on attachment 9114135 [details]
Bug 1601256 - Hide private browsing session permissions in preferences permission lists. r=johannh

Beta/Release Uplift Approval Request

  • User impact if declined: Permission lists in preferences show origin attributes, which is confusing for users.
    Example: https://bug1601256.bmoattachments.org/attachment.cgi?id=9113442
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See first bug comment
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): Medium risk, changes how the permission manager hands out permissions and could lead to regressions in components setting permissions in private browsing.
  • String changes made/needed:
Flags: needinfo?(pbz)
Attachment #9114135 - Flags: approval-mozilla-beta?
Attachment #9114134 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Verified on the latest Nightly version, 73.0a1 (2019-12-16). As per the discussion from the comments, the Private Browsing permission entries are now hidden and cleared upon closing the browser. Waiting for the fix to be uplifted in Beta.

Comment on attachment 9114135 [details]
Bug 1601256 - Hide private browsing session permissions in preferences permission lists. r=johannh

fix a regression with site permissions, verified in nightly, approved for 72.0b8

Attachment #9114135 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9114134 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+

I am attempting to verify this fix, but the steps to reproduce have changed because the settings done is Private browsing should be hidden in normal browsing mode and should expire along with the current session:

  1. Launch Firefox
  2. Open a new private window
  3. Open https://edition.cnn.com/videos
  4. Open the Site information panel
  5. Change block autoplay from Block Audio to Allow Audio and Video
  6. Make sure it applied by refreshing the CNN tab in a private browsing window and expecting the tab to autoplay video content with sound.
  7. Return to the non-private browsing (normal) window and make sure that the changes from the private browsing window did not stick to the non-private browsing window by opening the CNN page in a new tab and expecting block autoplay to have the default "Block Audio" setting.
  8. Restart the session and make sure that the previous block autoplay setting from private browsing mode has expired, by opening the CNN page in a private browsing window and expecting to see the default "Block Audio" setting in Site Information Panel.

Tested the following versions, which have the following results:

  • Nightly v74.0a1 from 2020-01-14: The issue appears fixed.
  • Beta v73.0b5: The issue appears fixed.
  • Release v72.0.1: The setting made in private browsing does also stick to the non-private browsing window; the setting is NOT hidden in the about:preferences/Permissions/Autoplay. On the other hand, at least the block autoplay setting set in private browsing did expire with the session.

@Paul: I cannot verify the firefox72 (release channel). I have tested this in Windows 10 and will verify others when this gets resolved. Can you have a look?

Flags: needinfo?(pbz)

Release should not be affected by this bug. It behaves differently in release because we do not separate permissions between private and normal browsing there yet. That's controlled by this pref. With isolation off, if you change the permission in private browsing it sets a normal non expiring permission that also affects the normal window.

Flags: needinfo?(pbz)

I have also verified this fix in Ubuntu 18 and Mac OS 10.14. Also, I am setting firefox72 as verified based on the comments above.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
See Also: → 1692776
Has Regression Range: no → yes
You need to log in before you can comment on or make changes to this bug.