2042309 - about:preferences#privacy is broken with MOZ_DATA_REPORTING false

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Pier Angelo Vendrame]

Attachment: Confusing gap left by the options when the first error is solved.

Some preferences aren't registered when MOZ_DATA_REPORTING is false (https://searchfox.org/firefox-main/rev/9d2be0cb8dbf1c3a7ef6ba79be35d637d0327eaa/browser/components/preferences/config/permissions-data.mjs#62-72).

However, that triggers this JS error:

Uncaught PreferenceNotAddedError: Setting "submitHealthReportBox" was unable to find Preference "datareporting.healthreport.uploadEnabled". Did you register it with Preferences.add/addAll?
    PreferenceNotAddedError chrome://global/content/preferences/Setting.mjs:185
    Setting chrome://global/content/preferences/Setting.mjs:254
    addSetting chrome://global/content/preferences/Preferences.mjs:101
    <anonymous> chrome://browser/content/preferences/config/permissions-data.mjs:350
    <anonymous> chrome://browser/content/preferences/privacy.js:13

An easy fix would be to register them regardless of MOZ_DATA_REPORTING and rely on their visible method not to show them.
That make almost all the other options visible again, but it leaves a confusing gap in the page, mostly due to the margin of the hidden options (see the attached screenshot).

STR:

Apply this diff:

diff --git a/browser/moz.configure b/browser/moz.configure
index 3ea3d88b9360..c55951b8debf 100644
--- a/browser/moz.configure
+++ b/browser/moz.configure
@@ -3,11 +3,11 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 imply_option("MOZ_PLACES", True)
-imply_option("MOZ_SERVICES_HEALTHREPORT", True)
+imply_option("MOZ_SERVICES_HEALTHREPORT", False)
 imply_option("MOZ_SERVICES_SYNC", True)
 imply_option("MOZ_DEDICATED_PROFILES", True)
 imply_option("MOZ_BLOCK_PROFILE_DOWNGRADE", True)
-imply_option("MOZ_NORMANDY", True)
+imply_option("MOZ_NORMANDY", False)
 imply_option("MOZ_PROFILE_MIGRATOR", True)
 
 

And set this in your mozconfig:

ac_add_options MOZ_TELEMETRY_REPORTING=
ac_add_options --disable-crashreporter

Alternatively, hardcode MOZ_DATA_REPORTING to False.

Comment 1

[:Gijs (he/him)]

Am I right in thinking this is a regression in 152 (as code appears to be from 1968118)? Also, you describe this as "partially broken" but I would actually be curious if any other settings still really work. JS exceptions in this type of location feel like they might break rather more - though it might depend on order of evaluation etc.

Comment 2

[Pier Angelo Vendrame]

Actually, we found the problem in 151, not in 152. The code got moved a lot between in 151 and 152 (we still don't know about 153, we wait for Firefox to go to beta to rebase Tor Browser, but sometimes we are late and check even later).
I wrote partially because the panel appeared to be partially populated, but now that I have actually tried it, I confirm that privacy/security settings don't work. So, yes, it's totally broken, not partially.
However, other panels (general, home, etc) seem to work correctly.

Comment 3

[:Gijs (he/him)]

[Tracking Requested - why for this release]:
Privacy pane being broken in tor browser feels like something we need to address immediately. Unclear to me if an appropriate fix could be made for 151 at this point; it sounds like the code is materially different so requesting tracking for 152 only for now.

Comment 4

[any1here]

Attachment: Bug 2042309 - Remove MOZ_DATA_REPORTING conditional and unneeded groupIds r=mstriemer

Comment 5

[Pier Angelo Vendrame]

(In reply to :Gijs (he/him) from comment #3)

[Tracking Requested - why for this release]:
Privacy pane being broken in tor browser feels like something we need to address immediately. Unclear to me if an appropriate fix could be made for 151 at this point; it sounds like the code is materially different so requesting tracking for 152 only for now.

Thank you, we really appreciate this!
However please take into account that we use Firefox RR only for our development channel, while we keep the stable channel in the ESR.
So, since this is not a supported Firefox configuration, for us it will be enough if this is solved in 153, and we can do the needed backports 🙂️.
Thanks again!

Comment 6

[any1here]

Attachment: Bug 2042309 - Remove MOZ_DATA_REPORTING conditional from preferences r=mstriemer

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/48fa603d9852
https://hg.mozilla.org/integration/autoland/rev/d5198237b6eb
Remove MOZ_DATA_REPORTING conditional from preferences r=hjones

Comment 8

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/d5198237b6eb

Comment 9

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:any1here, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox152 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Hanna Jones [:hjones]]

Created an uplift request just because the change seems very low risk/can only possibly fix things for a certain subset of users in 152

Comment 11

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: For certain users/under certain circumstances about:preferences is significantly broken due to preferences that were unnecessarily being added conditionally
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: This change can only fix things for this subset of users - unconditionally registering preferences shouldn't impact any other users
• String changes made/needed?: No
• Is Android affected?: no

Comment 12

[Hanna Jones [:hjones]]

Attachment: Bug 2042309 - Remove MOZ_DATA_REPORTING conditional from preferences

Original Revision: https://phabricator.services.mozilla.com/D303169

Comment 13

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/cc920de3a954
https://hg.mozilla.org/releases/mozilla-beta/rev/d9a6243e850a