970675 - Allow changing popup blocker settings for the session in private windows

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect, P3)

Description

[Koosha KM]

Attachment: Screenshot

While having 'Block pop-up windows' setting checked and doing private browsing, I am not able to unblock a blocked pop up window. The option 'Allow pop-ups for <URL>' is disabled.

Comment 1

[Josh Matthews [:jdm]]

This is intentional, since the way it is implemented makes it a permanent action regardless of private browsing status.

Comment 2

[Josh Matthews [:jdm]]

See bug 461625 for the original reasoning.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

This and bug 461625 look like they're kind of orthogonal to the reasoning in bug 967812, no? You're saying that we won't allow any permanent permission changes done from private windows but at the same time we mark bug 967812 as invalid due to bug 967812 comment #1?

Comment 4

[Koosha KM]

Well, at least incorporate some new feature to enable pop-ups temporarily for a single session of private browsing.

Comment 5

[(no longer active)]

(In reply to Tim Taubert [:ttaubert] from comment #3)
> This and bug 461625 look like they're kind of orthogonal to the reasoning in
> bug 967812, no? You're saying that we won't allow any permanent permission
> changes done from private windows but at the same time we mark bug 967812 as
> invalid due to bug 967812 comment #1?

The difference is that in this case it's not obvious to the user that they're granting a permission to the website.

That being said, I'd take a patch which makes this menu item work temporarily for the duration of the private window without touching the permission manager.

Comment 6

[Matthew N. [:MattN]]

(In reply to (no longer active) from comment #5)

I'd take a patch which makes this menu item work
temporarily for the duration of the private window without touching the
permission manager.

Johann, would you also take a patch for this? It will align better with how we handle other permission requests in private windows.

Comment 7

[Matthew N. [:MattN]]

Attachment: Bug 970675 - Allow changing popup blocker settings for the session in private windows. r=johannh

Comment 8

[Matthew N. [:MattN]]

Attachment: Bug 970675 - Hide private browsing session permissions in dialogs/permissions.js. r=johannh

Port Bug 1601256 to dialogs/permissions.js. Unfortunately these files forked even though they are very similar. This leads to bugs like this being introduced.

Depends on D118887

Comment 9

[Matthew N. [:MattN]]

Attachment: Bug 970675 - Add "popup" as a search keyword for pop-up blocking preferences. r=flod

Depends on D118888

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a1853ea9693
Allow changing popup blocker settings for the session in private windows. r=johannh
https://hg.mozilla.org/integration/autoland/rev/ec59b5b4094a
Hide private browsing session permissions in dialogs/permissions.js. r=johannh
https://hg.mozilla.org/integration/autoland/rev/98f78b0795b7
Add "popup" as a search keyword for pop-up blocking preferences. r=flod,preferences-reviewers,fluent-reviewers,Gijs

Comment 12

[Noemi Erli[:noemi_erli]]

Backed out 3 changesets (Bug 970675) for causing failures in browser_privatebrowsing_popupblocker.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=WSP7SNR6Rmi5qIlEPzfYaQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=98f78b0795b7c31970466c02149a56ad79da47e6

Log: https://treeherder.mozilla.org/logviewer?job_id=344964693&repo=autoland&lineNumber=4550

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

Comment 13

[Matthew N. [:MattN]]

Didn't realize there was a test for this there… looks like it can be deleted.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26054cb0248a
Allow changing popup blocker settings for the session in private windows. r=johannh
https://hg.mozilla.org/integration/autoland/rev/1b0b45679438
Hide private browsing session permissions in dialogs/permissions.js. r=johannh
https://hg.mozilla.org/integration/autoland/rev/71ed39c67e76
Add "popup" as a search keyword for pop-up blocking preferences. r=flod,preferences-reviewers,fluent-reviewers,Gijs

Comment 15

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/26054cb0248a
https://hg.mozilla.org/mozilla-central/rev/1b0b45679438
https://hg.mozilla.org/mozilla-central/rev/71ed39c67e76

Comment 16

[Pascal Chevrel (relman team) -> :pascalc]

Matt, Johann, Is that worth mentioning in our 92 nightly release notes?

Comment 17

[Matthew N. [:MattN]]

I would say probably not, it's a nice enhancement for sites where allowing a single popup is broken in private windows (e.g. they used the window reference returned by window.open) but most sites hopefully wouldn't have their legit popups blocked in the first place (ignoring bug 1611063 / bug 1656444).

Comment 18

[Johann Hofmann [:johannh]]

Agree with Matt, this is a small improvement :)

Comment 19

[Emma Zühlcke [:emz]]

Comment on attachment 9229096 [details]
Bug 970675 - Allow changing popup blocker settings for the session in private windows. r=johannh

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Allowing popups is partially broken for users in (permanent) private browsing mode. Can lead to site breakage in edge cases.
• User impact if declined: Users who are using permanent private browsing mode can't grant websites permission to open pop-ups. Instead they have to allow blocked pop-ups individually, which may break some website functionality. See discussion https://bugzilla.mozilla.org/show_bug.cgi?id=1740604#c21 (and following comments).
• Fix Landed on Version: 92
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small, well understood patch which has been in release since Firefox 92 without apparent regressions.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9229096 [details]
Bug 970675 - Allow changing popup blocker settings for the session in private windows. r=johannh

Actual functional changes are pretty minimal and the patch is well baked. Approved for 91.5esr.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/237b5e382ed7
https://hg.mozilla.org/releases/mozilla-esr91/rev/977fdafd14e9