Emil Ghitta, QA [:emilghitta] Reporter
	Description • 6 years ago

[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.

	:Felipe Gomes (needinfo for replies!)
	Comment 2 • 6 years ago

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?

	Emil Ghitta, QA [:emilghitta] Reporter
	Comment 3 • 6 years ago

(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.

	Mike Kaply [:mkaply] Assignee
	Comment 4 • 6 years ago

So should we trim the URL and use just the origin?

Is this a bug in the permissions manager?

	Mike Kaply [:mkaply] Assignee
	Comment 5 • 6 years ago

If you put a URL into preferences, it automatically gets truncated to the origin before being sent to permissions manager...

	:Felipe Gomes (needinfo for replies!)
	Comment 6 • 6 years ago

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.

	Mike Kaply [:mkaply] Assignee
	Comment 7 • 6 years ago

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.

	:Felipe Gomes (needinfo for replies!)
	Comment 8 • 6 years ago

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 [1] 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)


[1] https://searchfox.org/mozilla-central/source/toolkit/components/utils/JsonSchemaValidator.jsm#189

	Pulsebot
	Comment 10 • 6 years ago

Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/b3787ede5d7b
Show an error when full URL is used for permissions. r=Felipe

	Razvan Maries
	Comment 11 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/b3787ede5d7b

	Emil Ghitta, QA [:emilghitta] Reporter
	Comment 12 • 6 years ago

This is verified fixed using Firefox 65.0a1 (BuildId:20181120220133) on Windows 10 64bit, macOS 10.14 and Ubuntu 16.04 64bit.

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 6 years ago

Is this something that needs uplift consideration?

	Mike Kaply [:mkaply] Assignee
	Comment 14 • 6 years ago

Yes for 64, no for ESR.

	Mike Kaply [:mkaply] Assignee
	Comment 15 • 6 years ago

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:

	Julien Cristau [:jcristau]
	Comment 16 • 6 years ago

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

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 17 • 6 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/ec33377dad65

	Emil Ghitta, QA [:emilghitta] Reporter
	Comment 18 • 5 years ago

This is verified fixed using Firefox 64.0 (BuildId:20181206201918) on Windows 10 64bit, macOS 10.14 and Ubuntu 16.04 64bit.