No error is displayed if the "policies" string contains a typo inside the policies.json

VERIFIED FIXED in Firefox -esr60

Status

()

defect
P3
normal
VERIFIED FIXED
10 months ago
7 months ago

People

(Reporter: emilghitta, Assigned: nagy.ferenc.jr, Mentored)

Tracking

(Blocks 1 bug, {good-first-bug})

Trunk
Firefox 66
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 verified, firefox62 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 verified, firefox66 verified)

Details

(Whiteboard: [lang=js])

Attachments

(4 attachments)

Posted file policies.json
[Affected versions]:
Firefox 64.0a1 (BuildId:20181010100123).
Firefox 63.0b13 (BuildId:20181008155858).
Firefox 62.0.2 (BuildId:20180920131237).
Firefox 60.2.2esr (BuildId:20181001135620).

[Affected platforms]:
Windows 10 64bit.
macOS 10.13.6
Ubuntu 16.04 64bit.

[Preconditions]:
Enter a policy that contains a typo on the "policies" string (example: "policis") inside the policies.json file. 

[Steps to reproduce]:
1. Launch Firefox.
2. Access the about:support page and observe the "Enterprise Policies" information.
3. Access the about:policies page (available in Fx 64 and Fx 63).

[Expected result]:
Since the policies are not applied, an error is displayed in steps 2 and 3.

[Actual result]:
Step 2: The "Enterprise Policies" is displayed as Active.
Step 3: The about:polices#errors page is not displayed.

[Notes]
I have attached a policies.json files that reproduces this issue.
Mentor: felipc
Keywords: good-first-bug
Whiteboard: [lang=js]
Priority: -- → P4
I'd love to give this a go, but haven't done any work with firefox.

I have replicated the problem with the attached json file, and I realize this is a silly question, but what do I need to install to try and help fix this? 

Go through the Building Firefox on Windows (I'm on Windows 10, 64bit) to start? (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Windows_Prerequisites)
(In reply to estrauss from comment #1)
> I'd love to give this a go, but haven't done any work with firefox.
> 
> I have replicated the problem with the attached json file, and I realize
> this is a silly question, but what do I need to install to try and help fix
> this? 
> 
> Go through the Building Firefox on Windows (I'm on Windows 10, 64bit) to
> start?
> (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Windows_Prerequisites)

Hello there, thanks for volunteering!

Yeah, you should go through the Build instructions. Since for this bug you won't need to work on any C++ code (only JS), I suggest that you set up your builds using artifact builds (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds) because they will be much faster to work with (they use precompiled binaries and only repackages the JS/CSS files that were changed)
Hi Felipe, I believe this bug is narrow enough in scope to take on as a newbie. Would you say this is a GFB? if so I'd like to give it a try!
I already have a build of Firefox so I'll attempt to fix it straight away.
Hello Pablo, there's already a volunteer looking at this bug, but there are plenty others to take a look. I'm gonna suggest two in the Enterprise Policies component that are highly desired and would be awesome for someone to work on: bug 1502134 and bug 1485193.

(the first one that I mentioned is narrower, so it's a good starting point!)
Assignee: nobody → estrauss
Priority: P4 → P3
Sorry, I got bogged down trying to find a job (fun fun fun), if you want to assign it to someone else, I completely understand, otherwise I will try to get to it asap (maybe a week or so at least).
Ok, no problem! I'll leave the bug unassigned in case someone else wants to take it. If you want to contribute later you can always see if this bug is still open or choose a new one :)
Assignee: estrauss → nobody
(In reply to Pablo Lluch from comment #3)
> Hi Felipe, I believe this bug is narrow enough in scope to take on as a
> newbie. Would you say this is a GFB? if so I'd like to give it a try!
> I already have a build of Firefox so I'll attempt to fix it straight away.

nice
I'm not sure if anyone is working on it right now, I would like to pick this up.
Assignee: nobody → nagy.ferenc.jr
Status: NEW → ASSIGNED
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/425ac2d14a27
Make about:policies report an error if json doesn't contain a 'policies' object. r=felipe
Backed out 2 changesets (bug 1497928, bug 1494598) for browser-chrome failures at browser/components/enterprisepolicies/tests/browser/browser_policies_macosparser_unflatten.js 

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/afab7f523f072f350733bc3e5ce96a0e76663cf1

Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=58cd47fe1774e23092d636e32c6afd867fa7fd47

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=217765293&repo=mozilla-inbound&lineNumber=3138

task 2018-12-18T21:12:40.819Z] 21:12:40     INFO - TEST-START | browser/components/enterprisepolicies/tests/browser/browser_policies_macosparser_unflatten.js
[task 2018-12-18T21:12:40.827Z] 21:12:40     INFO - TEST-INFO | started process screentopng
[task 2018-12-18T21:12:41.683Z] 21:12:41     INFO - TEST-INFO | screentopng: exit 0
[task 2018-12-18T21:12:41.683Z] 21:12:41     INFO - TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/browser_policies_macosparser_unflatten.js | Exception thrown - [Exception... "File error: Not found"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: chrome://mochitests/content/browser/browser/components/enterprisepolicies/tests/browser/browser_policies_macosparser_unflatten.js :: <TOP_LEVEL> :: line 6"  data: no]
Flags: needinfo?(felipc)
The problem here was that the new test was added in browser.ini in the spot from the `skip-if` settings of another test that is only supposed to run on mac:
https://hg.mozilla.org/integration/mozilla-inbound/rev/425ac2d14a27#l2.12
Flags: needinfo?(felipc)
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e15dc2bf340
Make about:policies report an error if json doesn't contain a 'policies' object. r=felipe
https://hg.mozilla.org/mozilla-central/rev/8e15dc2bf340
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Is this something we should consider for backport?
Yeah, I guess this is simple and it helps people to debug policies problems
Flags: needinfo?(felipc)
Comment on attachment 9028933 [details]
Bug 1497928 - Fixing about:policies to report error when 'policies' object is not present in policies.json r=Felipe

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Enterprise Policies

User impact if declined: This just adds a helpful message if there's a typo in the policies.json file

Is this code covered by automated tests?: Yes

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 just adds a check for a mandatory property in the policies.json file and display a warning if it's not there, instead of silently failing

String changes made/needed: none
Attachment #9028933 - Flags: approval-mozilla-beta?
Comment on attachment 9028933 [details]
Bug 1497928 - Fixing about:policies to report error when 'policies' object is not present in policies.json r=Felipe

[Triage Comment]
Adds a helpful message if there's a typo in the policies.json file. Approved for 65.0b7.
Attachment #9028933 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This grafts cleanly to ESR60 also - should we take it there too?
Flags: needinfo?(felipc)
On one hand, about:policies is not present on ESR, so it's not gonna be highly visible.. On the other hand, this will still spew the error message to the console, and say that Enterprise Policies is in a "Failed" state in about:support.. So I think it will in general be useful for ESR.
Flags: needinfo?(felipc)
Comment on attachment 9028933 [details]
Bug 1497928 - Fixing about:policies to report error when 'policies' object is not present in policies.json r=Felipe

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Help debug a potential type in policies.json

User impact if declined: Harder to diagnose why policies deployment might not be working

Fix Landed on Version: 66, uplifted to 65

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Simple check in the Enterprise Policies code

String or UUID changes made by this patch: none
Attachment #9028933 - Flags: approval-mozilla-esr60?
Comment on attachment 9028933 [details]
Bug 1497928 - Fixing about:policies to report error when 'policies' object is not present in policies.json r=Felipe

Avoids confusing messages in the error console and about:support about Enterprise Policies being in a failed state. Approved for 60.5.0esr.
Attachment #9028933 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Would be good to get a quick QA pass on this too just to make sure everything's working as expected.
Flags: qe-verify+
This issue is verified fixed using Firefox 65.0b8 (BuildId:20190103150357), Firefox 66.0a1 (BuildID:20190106214444) and Firefox 60.4.1esr (provided in comment 27) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.