1591748 - "Exceptions - Cookies and Site Data" are ignored when setting "Always use private browsing mode".

Status: RESOLVED FIXED

(Core :: Permission Manager, defect, P2)

Description

[Eiichi]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:72.0) Gecko/20100101 Firefox/72.0

Steps to reproduce:

1. Open "about:preferences#privacy" -> "Manage Permissions…" .
2. Set "https://bugzilla.mozilla.org/home" to "Allow for Session" and save.
3. Set "Always use private browsing mode" in "Privacy & Security" preferences.
4. Open "https://bugzilla.mozilla.org/home" and try to log in.

Actual results:

"Allow for Session" is ignored and I can't log in.

Expected results:

"Allow for Session" is not ignored and I can log in as Firefox Developer Edition 71.0b4 can do.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Eiichi]

Attachment: Screenshots.png

Screenshots of Site Information Panel

Comment 3

[Johann Hofmann [:johannh]]

Ah, the cookie exceptions problem again, this time for privateBrowsingId. This is bug 1556212, but it might make sense to track separately. What happens here is that we added support for more origin attributes for permissions, but the cookies permission is added without OAs in about:preferences.

Comment 4

[Johann Hofmann [:johannh]]

FWIW setting permissions.isolateBy.privateBrowsing to false in about:config should fix this for you momentarily.

Comment 5

[Eiichi]

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

FWIW setting permissions.isolateBy.privateBrowsing to false in about:config should fix this for you momentarily.

Thanks for your a quick help.
I confirmed that setting permissions.isolateBy.privateBrowsing to false has fixed this issue.

Comment 6

[Emma Zühlcke [:emz]]

I'd like to reuse this patch originally planned for a similar FPI bug: https://phabricator.services.mozilla.com/D53191, but only have it strip private browsing / user context, not FPI.
Just making sure, since this doesn't touch FPI origin attributes, would it be ok from the tor browse side, Tom?

Comment 7

[Tom Ritter [:tjr]]

Yes, that should be fine, thanks for checking.

Comment 8

[Emma Zühlcke [:emz]]

Attachment: Bug 1591748 - nsPermissionMgr: Added principal oa strip permission list for userContext and privateBrowsing. r=ehsan

Comment 9

[Emma Zühlcke [:emz]]

Attachment: Bug 1591748 - Added test for oa strip permission list. r=ehsan

Depends on D58831

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/94d8942709df
nsPermissionMgr: Added principal oa strip permission list for userContext and privateBrowsing. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/18fe0c505632
Added test for oa strip permission list. r=Ehsan

Comment 11

[Noemi Erli[:noemi_erli]]

Backed out 2 changesets (Bug 1591748) for causing bustages in nsPermissionManager.cpp CLOSED TREE

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=283736382&resultStatus=testfailed%2Cbusted%2Cexception&revision=18fe0c505632b22d794dfb1e78a6fba28ec2aa81

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=283736417&repo=autoland&lineNumber=24331

Backout: https://hg.mozilla.org/integration/autoland/rev/de2022bc51bb352ea750f211da08e2feb7a77939

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9902221ac174
nsPermissionMgr: Added principal oa strip permission list for userContext and privateBrowsing. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/34dd9b331df3
Added test for oa strip permission list. r=Ehsan

Comment 13

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/9902221ac174
https://hg.mozilla.org/mozilla-central/rev/34dd9b331df3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Is the user impact of this bug severe enough that we should consider uplifting this fix to Beta for Fx73 or can this ride Fx74 to release? Please nominate the patch for approval if you feel it should be uplifted. It grafts cleanly as-landed.

Comment 15

[Emma Zühlcke [:emz]]

Thanks Ryan! I don't think we need to uplift this. The issue only affects pre-release channels and is rather an edge case.