Closed
Bug 735557
Opened 13 years ago
Closed 13 years ago
Add about:preferences to the inContentWhitelist
Categories
(Firefox :: Settings UI, defect)
Firefox
Settings UI
Tracking
()
RESOLVED
FIXED
Firefox 15
People
(Reporter: jaws, Assigned: jon.rietveld)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Unfocused
:
review+
jaws
:
feedback+
|
Details | Diff | Splinter Review |
The in-content preferences should be rendered the same way as about:addons, without the navigation toolbar.
To do this
1) Add a testcase for about:preferences to test_disablechrome.js (http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_disablechrome.js#141). This is similar to the work that was done for bug 716464.
2) Run the test to make sure that the test fails for about:preferences.
3) Add about:preferences to the inContentWhitelist here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#4425
4) Run the test to make sure that the test passes for about:preferences.
Assignee | ||
Comment 1•13 years ago
|
||
In this patch I believe I have the test failing correctly.
Attachment #606105 -
Flags: feedback?(jwein)
Attachment #606105 -
Flags: feedback?(bmcbride)
Reporter | ||
Comment 2•13 years ago
|
||
Comment on attachment 606105 [details] [diff] [review]
in-content preferences disable toolbar test
Review of attachment 606105 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good.
Attachment #606105 -
Flags: feedback?(jwein) → feedback+
Assignee | ||
Comment 3•13 years ago
|
||
I have added about:preferences to the inContentWhitelist, but the new tests are still failing and I cannot figure out why. Any feedback would be greatly appreciated!
Attachment #606105 -
Attachment is obsolete: true
Attachment #606105 -
Flags: feedback?(bmcbride)
Attachment #606407 -
Flags: feedback?(jwein)
Attachment #606407 -
Flags: feedback?(bmcbride)
Assignee | ||
Comment 4•13 years ago
|
||
Everything appears to be working correctly! All tests pass :)
Attachment #606407 -
Attachment is obsolete: true
Attachment #606407 -
Flags: feedback?(jwein)
Attachment #606407 -
Flags: feedback?(bmcbride)
Attachment #606416 -
Flags: feedback?(jwein)
Attachment #606416 -
Flags: feedback?(bmcbride)
Reporter | ||
Comment 5•13 years ago
|
||
Comment on attachment 606416 [details] [diff] [review]
remove toolbar for in-content preferences
Review of attachment 606416 [details] [diff] [review]:
-----------------------------------------------------------------
Gah, so many whitespace changes! They are for the better though, so we could undo the whitespace fixes or leave them. I'm fine with the fixes staying. r=me with Blair's approval.
Attachment #606416 -
Flags: feedback?(jwein) → feedback+
Assignee | ||
Comment 6•13 years ago
|
||
yea, Blair said the same thing, duno how those happened :/
Comment 7•13 years ago
|
||
Comment on attachment 606416 [details] [diff] [review]
remove toolbar for in-content preferences
Review of attachment 606416 [details] [diff] [review]:
-----------------------------------------------------------------
Lots of unrelated whitespace changes in browser.js :(
::: browser/base/content/browser.js
@@ +4422,5 @@
> startTime: 0,
> statusText: "",
> isBusy: false,
> + inContentWhitelist: ["about:addons", "about:permissions",
> + "about:sync-progress", "about:preferences"],
Indent the second line to be lined up with the first item in the array on the first line.
::: browser/base/content/test/browser_disablechrome.js
@@ +195,5 @@
> + test_url(HTTPSRC + "disablechrome.html", false, run_chrome_about_preferences_test_5);
> +}
> +
> +// Should not hide the chrome
> +function run_chrome_about_preferences_test_5() {
Keep the function naming consistent with the rest of the file - run_chrome_about_test_5.
Ditto for run_chrome_about_test_6.
Attachment #606416 -
Flags: feedback?(jwein)
Attachment #606416 -
Flags: feedback?(bmcbride)
Attachment #606416 -
Flags: feedback+
Comment 8•13 years ago
|
||
(In reply to Jared Wein [:jaws] from comment #5)
> They are for the better though, so we could
> undo the whitespace fixes or leave them.
I'd rather not have them land with this, on the odd change that this gets backed out at any stage (unnecessary churn in related code).
Comment 9•13 years ago
|
||
Comment on attachment 606416 [details] [diff] [review]
remove toolbar for in-content preferences
(Stupid bugzilla...)
Attachment #606416 -
Flags: feedback?(jwein) → feedback+
Assignee | ||
Comment 10•13 years ago
|
||
Fixed naming issues and removed unrelated whitespace from browser.js :)
Attachment #606416 -
Attachment is obsolete: true
Attachment #606571 -
Flags: feedback?(jwein)
Attachment #606571 -
Flags: feedback?(bmcbride)
Reporter | ||
Comment 11•13 years ago
|
||
Comment on attachment 606571 [details] [diff] [review]
remove toolbar for in-content preferences
Review of attachment 606571 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good. r=me with Blair's approval :P
Attachment #606571 -
Flags: feedback?(jwein) → feedback+
Updated•13 years ago
|
Attachment #606571 -
Flags: feedback?(bmcbride) → review+
Reporter | ||
Comment 12•13 years ago
|
||
Target Milestone: --- → Firefox 15
Comment 13•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•