Closed
Bug 1497928
Opened 6 years ago
Closed 5 years ago
No error is displayed if the "policies" string contains a typo inside the policies.json
Categories
(Firefox :: Enterprise Policies, defect, P3)
Firefox
Enterprise Policies
Tracking
()
VERIFIED
FIXED
Firefox 66
People
(Reporter: emilghitta, Assigned: nagy.ferenc.jr, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [lang=js])
Attachments
(4 files)
204 bytes,
application/json
|
Details | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr60+
|
Details | Review |
[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.
Updated•6 years ago
|
Updated•6 years ago
|
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)
Comment 2•6 years ago
|
||
(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)
Comment 3•5 years ago
|
||
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.
Comment 4•5 years ago
|
||
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!)
Updated•5 years ago
|
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).
Comment 6•5 years ago
|
||
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
Assignee | ||
Comment 8•5 years ago
|
||
I'm not sure if anyone is working on it right now, I would like to pick this up.
Assignee | ||
Comment 9•5 years ago
|
||
Updated•5 years ago
|
Assignee: nobody → nagy.ferenc.jr
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•5 years ago
|
||
Assignee | ||
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
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
Comment 13•5 years ago
|
||
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)
Comment 14•5 years ago
|
||
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)
Comment 15•5 years ago
|
||
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
Comment 16•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8e15dc2bf340
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Comment 17•5 years ago
|
||
Is this something we should consider for backport?
status-firefox65:
--- → affected
Flags: needinfo?(felipc)
Comment 18•5 years ago
|
||
Yeah, I guess this is simple and it helps people to debug policies problems
Flags: needinfo?(felipc)
Comment 19•5 years ago
|
||
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 20•5 years ago
|
||
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+
Comment 21•5 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/a5ccc1d7c95f
Flags: in-testsuite+
Comment 22•5 years ago
|
||
This grafts cleanly to ESR60 also - should we take it there too?
Flags: needinfo?(felipc)
Comment 23•5 years ago
|
||
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 24•5 years ago
|
||
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 25•5 years ago
|
||
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+
Comment 26•5 years ago
|
||
Would be good to get a quick QA pass on this too just to make sure everything's working as expected.
Flags: qe-verify+
Comment 27•5 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/4fcdddf6835e
Reporter | ||
Comment 28•5 years ago
|
||
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.
Description
•