Closed Bug 1507223 Opened 1 year ago Closed 1 year ago
The "Allow" argument for the Popup
Blocking policy is not working
[Affected versions]: Firefox 65.0a1 (BuildId:20181114100226) Firefox 64.0b9 (BuildId:20181112164519) Firefox 63.0.1 (BuildId:20181030165643) Firefox 60.3.0esr (BuildId:20181017185317) [Affected platforms]: Windows 10 64bit. macOS 10.14. Ubuntu 16.04 64bit. [Preconditions]: Place the attached .json file inside the "distribution" folder or allow PopupBlocking on several websites via GPO. [Steps to reproduce]: 1. Launch Firefox. 2. Access the about:policies page. 3. Access the allowed websites specified inside the policy. [Expected result]: 2. The policy is successfully displayed inside the about:policies page. 3. The following message is not being displayed :"Firefox prevented this site from opening pop-up windows". [Actual result]: 2. The PopupBlocking policy is not being displayed inside the about:policies page. 3. The "Firefox prevented this site from opening pop-up windows" string is displayed. [Regression range]: I don't think this is a regression.
The PopupBlocking policy only accepts an origin (i.e. http/https:// + domain), it _can't_ be used with a full page URL like in this file. Was there any error displayed in about:policies?
(In reply to :Felipe Gomes (needinfo me!) from comment #2) > The PopupBlocking policy only accepts an origin (i.e. http/https:// + > domain), it _can't_ be used with a full page URL like in this file. Indeed, it works only if the origin is specified. > Was there any error displayed in about:policies? There are no errors displayed inside the about:policies page if the full page URL is specified.
So should we trim the URL and use just the origin? Is this a bug in the permissions manager?
If you put a URL into preferences, it automatically gets truncated to the origin before being sent to permissions manager...
I think we should trim and log a warning saying that only the origin should be used. It's not a bug in the permissions manager, it's the intended behavior. It actually supports both origin-only or full paths, and it depends on the specific permission. The popup permission works with origins.
The schemas all say origin, so this is just getting ignored. Since this is a pretty common thing to do, I think we should update the schema to allow origin and URL and then truncate URLs.
The intention of the "origin" type is to avoid any confusion here, because it might be dangerous that someone think it's just granting the permission for one unique URL, but it's actually granting the permission for the entire origin. This used to cause a warning before, as any wrong item in an array type would cause the entire policy to fail validation. Now that is no longer true, this actually doesn't trigger the "Invalid Policy" error msg anymore. So I don't think we should add a URL to the schema. What we can do is in make the origin validator  to emit a warning about it. And the `origin` validator itself could do the truncation together with this warning. (I'd prefer that it didn't truncate, only logged the error, but I could be convinced otherwise if you think it's very important)  https://searchfox.org/mozilla-central/source/toolkit/components/utils/JsonSchemaValidator.jsm#189
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/b3787ede5d7b Show an error when full URL is used for permissions. r=Felipe
Assignee: nobody → mozilla
This is verified fixed using Firefox 65.0a1 (BuildId:20181120220133) on Windows 10 64bit, macOS 10.14 and Ubuntu 16.04 64bit.
Is this something that needs uplift consideration?
Yes for 64, no for ESR.
Comment on attachment 9025799 [details] Bug 1507223 - Show an error when full URL is used for permissions. [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: None User impact if declined: Admins don't know why popup errors weren't correct. Is this code covered by automated tests?: No Has the fix been verified in Nightly?: No Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): This is extremely low risk. Just adding an error message for this case. String changes made/needed:
Attachment #9025799 - Flags: approval-mozilla-beta?
Comment on attachment 9025799 [details] Bug 1507223 - Show an error when full URL is used for permissions. better diagnostics for policy config issue, approved for 64.0b14
Attachment #9025799 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This is verified fixed using Firefox 64.0 (BuildId:20181206201918) on Windows 10 64bit, macOS 10.14 and Ubuntu 16.04 64bit.
You need to log in before you can comment on or make changes to this bug.