Closed
Bug 1355522
Opened 8 years ago
Closed 8 years ago
"Reports" section should be under Privacy&Security, not under Updates
Categories
(Firefox :: Settings UI, defect)
Firefox
Settings UI
Tracking
()
VERIFIED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | verified |
People
(Reporter: Felipe, Assigned: jaws)
References
Details
Attachments
(2 files)
With the reorganized Preferences tab, the Reports section (aka fhr, telemetry & crashes) are under the Updates section, which doesn't seem the best place for it.
The spec confirms that it should be under Privacy: https://mozilla.invisionapp.com/share/5RA0R4HAE#/screens/214906112
Also, of note: on a first run, there's an infobar that shows informing the use about Telemetry.. And it has a button that says "Choose What I Share". That button is currently taking to the Updates section and will need to be updated too.
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(jaws)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8857513 [details]
Bug 1355522 - Move Telemetry and Health Report prefs to Privacy & Security section.
https://reviewboard.mozilla.org/r/129472/#review133024
::: browser/components/preferences/in-content/privacy.js:1343
(Diff revision 1)
> + /**
> + * Set the status of the telemetry controls based on the input argument.
> + * @param {Boolean} aEnabled False disables the controls, true enables them.
> + */
> + setTelemetrySectionEnabled(aEnabled) {
> + if (AppConstants.MOZ_TELEMETRY_REPORTING) {
If this was new code, I would suggest an early return, to avoid indenting the whole method.
I don't expect you to do this change, because I know this is just code being moved around. But if you decide to do it, please do it for all the methods currently following the same pattern.
::: browser/components/preferences/in-content/privacy.js:1385
(Diff revision 1)
> + Services.prefs.setBoolPref(PREF_UPLOAD_ENABLED, checkbox.checked);
> + this.setTelemetrySectionEnabled(checkbox.checked);
> + }
> + },
> +
> - // Methods for Offline Apps (AppCache)
> + // Methods for Offline Apps (AppCache)
nit: I don't think you intended to add one space before this comment.
Attachment #8857513 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Attachment #8857513 -
Flags: review?(mconley)
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8857514 [details]
Bug 1355522 - Move scripts for spell-checking, hardware acceleration, and on-screen keyboards to main.js since the prefs moved to the main category.
https://reviewboard.mozilla.org/r/129474/#review133026
::: commit-message-4b36b:1
(Diff revision 1)
> +Bug 1355522 - Move scripts for spell-checking, hardware acceleration, and on-screen keyboardsto main.js since the prefs moved to the main category. r?mconley
"keyboardsto" please fix the typo in the commit message.
::: browser/components/preferences/in-content/tests/browser_checkspelling.js:15
(Diff revision 1)
> + let checkbox = doc.querySelector("#checkSpelling");
> + is(checkbox.checked,
> + Services.prefs.getIntPref("layout.spellcheckDefault") == 2,
> + "checkbox should represent pref value before clicking on checkbox");
> +
> + checkbox.click();
Should the test ensure that the value of checkbox.checked is different after calling checkbox.click()?
Attachment #8857514 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Attachment #8857514 -
Flags: review?(mconley)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8857513 [details]
Bug 1355522 - Move Telemetry and Health Report prefs to Privacy & Security section.
https://reviewboard.mozilla.org/r/129472/#review133024
> If this was new code, I would suggest an early return, to avoid indenting the whole method.
>
> I don't expect you to do this change, because I know this is just code being moved around. But if you decide to do it, please do it for all the methods currently following the same pattern.
I've fixed this in the patch here and other places within this file.
Assignee | ||
Comment 10•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8857514 [details]
Bug 1355522 - Move scripts for spell-checking, hardware acceleration, and on-screen keyboards to main.js since the prefs moved to the main category.
https://reviewboard.mozilla.org/r/129474/#review133026
> Should the test ensure that the value of checkbox.checked is different after calling checkbox.click()?
Yes, I added a check for the value of checkbox.checked to the test.
Comment 11•8 years ago
|
||
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f142622c5a2b
Move Telemetry and Health Report prefs to Privacy & Security section. r=florian
https://hg.mozilla.org/integration/autoland/rev/0700969e50a4
Move scripts for spell-checking, hardware acceleration, and on-screen keyboards to main.js since the prefs moved to the main category. r=florian
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f142622c5a2b
https://hg.mozilla.org/mozilla-central/rev/0700969e50a4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment 13•8 years ago
|
||
I have reproduced this bug with Nightly 55.0a1 (2017-04-11) (64-bit) on Ubuntu 16.04 LTS!
This bug's fix is verified with latest Nightly!
Build ID : 20170419100228
User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
[bugday-20170419]
Assignee | ||
Updated•8 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•