Closed Bug 804430 Opened 7 years ago Closed 6 years ago

about:support should list locked prefs

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: Unfocused, Assigned: rittme)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js])

Attachments

(1 file, 1 obsolete file)

about:support currently lists:
* modified prefs (based on a whitelist)
* presence of a user.js

It should also list which prefs are locked, as these can lead to unexpected results.

Should be enough to go through the existing whitelist of prefs, checking nsIPrefBranch.prefIsLocked()
OS: Windows 7 → All
Hardware: x86_64 → All
Whiteboard: [good first bug]
Unfocused, would you be willing to mentor a user through this?
Flags: needinfo?(bmcbride)
Yep!

Here's a recent changeset that adds a section to about:support - it's a good example to learn by:
http://hg.mozilla.org/mozilla-central/rev/a6f66d193b79
Flags: needinfo?(bmcbride)
Whiteboard: [good first bug] → [good first bug][mentor=bmcbride@mozilla.com][lang=js]
I'm new to firefox development and would like to work on this bug to get started. 
Could I be assigned to it?
Thank you
Bernardo, that would be great! Please go ahead and work on it, and feel free to comment in this bug to ask Unfocused any questions you might have. As soon as you've got a first try at a patch ready to go, we'll officially assign the bug to you.

If you haven't got your development environment set up yet, our developer guide's Getting Started section - linked from here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide - is very helpful.

Good luck!
So, here is my first try at a patch for this bug. 
I hope this works as expected.
Attachment #8434844 - Flags: review?(bmcbride)
Assignee: nobody → bernardo
Status: NEW → ASSIGNED
Comment on attachment 8434844 [details] [diff] [review]
rev1 - Added list of locked prefs to about:support

Review of attachment 8434844 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, your first patch and it gets an instant r+ :)
Attachment #8434844 - Flags: review?(bmcbride) → review+
Pushed to the fx-team branch, which gets merged into mozilla-central a couple of times a day.

 https://hg.mozilla.org/integration/fx-team/rev/84a0967ba12d
Sorry, had to back this out due to m-bc3 failures:

https://hg.mozilla.org/integration/fx-team/rev/17402b39be24

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/modules/tests/browser/browser_Troubleshoot.js | Error: Validation error: Object has property not in schema: object={"application":{"name":"Firefox","version":"32.0a1","userAgent":"Mozilla/5.0 (X11; Linux i686; rv:32.0) Gecko/20100101 Firefox/32.0","supportURL":"https://support.mozilla.org/1/firefox/32.0a1/Linux/en-US/"},"crashes":{"submitted":[],"pending":0},"modifiedPreferences":{"accessibility.typeaheadfind.flashBar":0,"accessi... [exceeded max length]
Oops :\ Forgot that test was that strict.


Bernardo: you'll need to update the browser/toolkit/modules/tests/browser/browser_Troubleshoot.js test. It's not safe to lock prefs in a test, but we need to at least modify the expected schema to account for the new data.
Unfocused: I have updated the test scheme as you suggested. I don't know if it's enough, as I don't know how to run the test that failed :(
Thank you for your help!
Attachment #8434844 - Attachment is obsolete: true
Attachment #8437808 - Flags: review?(bmcbride)
(In reply to Bernardo Rittmeyer [:rittme] from comment #10)
> Unfocused: I have updated the test scheme as you suggested. I don't know if
> it's enough, as I don't know how to run the test that failed :(

The command to run the test is:
./mach mochitest-browser toolkit/modules/tests/browser/browser_Troubleshoot.js

the "browser_" prefix on the test indicates it's a browser-chrome test and those are run with the ./mach mochitest-browser command. https://developer.mozilla.org/en-US/docs/Browser_chrome_tests
(In reply to Matthew N. [:MattN] from comment #11)
> The command to run the test is:
> ./mach mochitest-browser
> toolkit/modules/tests/browser/browser_Troubleshoot.js
> 
> the "browser_" prefix on the test indicates it's a browser-chrome test and
> those are run with the ./mach mochitest-browser command.
> https://developer.mozilla.org/en-US/docs/Browser_chrome_tests

Cool, thank you for the info.
So I've tested it and it seems to work just fine now. It passes the test with no problems.
Comment on attachment 8437808 [details] [diff] [review]
rev2 - Added list of locked prefs to about:support (with test scheme updated)

Review of attachment 8437808 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome, thanks :)
Attachment #8437808 - Flags: review?(bmcbride) → review+
Has this been through Try to verify that the previous failures are fixed? If so, please provide a link :)
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/b6ad6f0ee9de
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js] → [good first bug][mentor=bmcbride@mozilla.com][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/b6ad6f0ee9de
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js][fixed-in-fx-team] → [good first bug][mentor=bmcbride@mozilla.com][lang=js]
Target Milestone: --- → mozilla33
This duplicated the getPref helper used just above, which isn't ideal - can fix in bug 1034724.
You need to log in before you can comment on or make changes to this bug.