2043551 - Settings-redesign: DNS over HTTPS setting for 2 vs 3 inversed

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Manuel Bucher [:manuel]]

Attachment: settings.png

The text checkbox for doh mode 2 vs 3 reads opposite.

The previous "Max protection: Nightly will always use secure DNS. You’ll see a security risk warning before we use your system DNS." (network.trr.mode 3) now maps to "Custom" with the unchecked option "[ ] Always warn me if secure DNS isn’t available".

The previous "Increased Protection" (network.trr.mode 2) now maps to "Custom" with the checked option "[x] Always warn me if secure DNS isn’t available".

It seems like the checkbox is backwards. I'm attaching screenshots of how the settings look like. First row is network.trr.mode 2, second row is network.trr.mode 3

Expected result: The checked state of the checkbox maps to doh mode 3, the unchecked state maps to doh mode 2.

Comment 1

[Valentin Gosu [:valentin]]

Post regressor bug

Comment 2

[Valentin Gosu [:valentin]]

I think this is caused by the code here:
privacy.mjs
At first look it doesn't seem obviously wrong, but it's interesting that it doesn't have any code coverage.

Comment 3

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1971428

:bvandersloot, since you are the author of the regressor, bug 1971428, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Manuel Bucher [:manuel]]

This is disabled on Fx151 (per bug 2028609).

Comment 5

[Benjamin VanderSloot [:bvandersloot]]

Yikes. Looks like when I was implementing this checkbox, there was some back and forth on the directionality of the checkbox's purpose. I got mixed up in my negative counting. Patch incoming.

Comment 6

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 2043551 - Fix DNS over HTTPS setting inversion of mode 2 and 3 - r=manuel!

Comment 7

[Benjamin VanderSloot [:bvandersloot]]

This probably needs a beta uplift as well.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/67fe6fde4821
https://hg.mozilla.org/integration/autoland/rev/24d290366ae2
Fix DNS over HTTPS setting inversion of mode 2 and 3 - r=manuel,mconley

Comment 9

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/24d290366ae2

Comment 10

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 2043551 - Fix DNS over HTTPS setting inversion of mode 2 and 3 -

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

Comment 11

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: A setting string for DNS over HTTPS will say the opposite of what the checkbox does
• Code covered by automated testing?: yes
• 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: Small change.
• String changes made/needed?: No
• Is Android affected?: no

Comment 12

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/ff906aadfbf8
https://hg.mozilla.org/releases/mozilla-beta/rev/4cd06b21f3ca