add restore default value to DNS-over-HTTPS preferences

VERIFIED FIXED in Firefox 65

Status

()

enhancement
P1
normal
VERIFIED FIXED
9 months ago
5 months ago

People

(Reporter: ewright, Assigned: ewright)

Tracking

64 Branch
Firefox 65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 wontfix, firefox65 verified)

Details

(Whiteboard: [trr])

Attachments

(4 attachments)

Assignee

Description

9 months ago
If a user edits the url field under "Enable DNS over HTTPS", they have no easy way to restore the url to the default value.
We'd like to add a button to restore the default value. Mocks from :rfeeley are attached.
Assignee

Comment 1

9 months ago
Assignee

Comment 3

9 months ago
This adds a button to the preferences UI to restore the default value for network.trr.uri preference.
Assignee

Comment 4

9 months ago
:sfoster, this also changes the "example" url in the placeholder to the actual url "https://mozilla.cloudflare-dns.com/dns-query" as requested by :jduell.

note: This is my first time using phabricator, so please let me know if I should change anything in its set up/commit message.
Status: NEW → ASSIGNED
Priority: -- → P1

Comment 5

9 months ago
I just wanted to chime in here to note that we're officially OK with making the default URI the Cloudflare one (for now: we'll change this UI as we get more DoH providers).  So ignore bug 1482271 comment 12.
QA Contact: jaws
Whiteboard: [trr]
QA Contact: jaws

Comment 6

9 months ago
Pushed by ewright@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74a98c04cd54
Add a button to restore the default value for network.trr.uri preference. r=flod,jaws
Backed out changeset 74a98c04cd54 (bug 1495583) for browser chrome failures on browser_connection_dnsoverhttps

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success,testfailed,busted,exception&group_state=expanded&revision=74a98c04cd54385d99b2a49842df0a9f14846bf4&searchStr=chrome&selectedJob=205193460

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=205193460&repo=autoland&lineNumber=2132

Backout link: https://hg.mozilla.org/integration/autoland/rev/4e77c1c74676244514aff0fd213fb77ad18f371d

[task 2018-10-12T23:02:16.475Z] 23:02:16     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | Actual value of network.trr.mode matches expected default value - 
[task 2018-10-12T23:02:16.475Z] 23:02:16     INFO - Buffered messages finished
[task 2018-10-12T23:02:16.476Z] 23:02:16     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | Actual value of network.trr.mode matches expected default value - Got https://mozilla.cloudflare-dns.com/dns-query, expected 
[task 2018-10-12T23:02:16.477Z] 23:02:16     INFO - Stack trace:
[task 2018-10-12T23:02:16.478Z] 23:02:16     INFO - chrome://mochikit/content/browser-test.js:test_is:1295
[task 2018-10-12T23:02:16.479Z] 23:02:16     INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js:default_values:124
[task 2018-10-12T23:02:16.480Z] 23:02:16     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1093
[task 2018-10-12T23:02:16.481Z] 23:02:16     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1084
[task 2018-10-12T23:02:16.481Z] 23:02:16     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:986
[task 2018-10-12T23:02:16.482Z] 23:02:16     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
[task 2018-10-12T23:02:16.484Z] 23:02:16     INFO - Leaving test bound default_values
[task 2018-10-12T23:02:16.485Z] 23:02:16     INFO - Entering test bound testVariation
[task 2018-10-12T23:02:17.454Z] 23:02:17     INFO - 1: testWithProperties: testing with {"expectedModePref":0,"expectedUriValue":""}
[task 2018-10-12T23:02:18.084Z] 23:02:18     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | Check the proper URL is loaded - 
[task 2018-10-12T23:02:18.086Z] 23:02:18     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | Element should not be null, when checking visibility - 
[task 2018-10-12T23:02:18.091Z] 23:02:18     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | Overlay is visible - 
[task 2018-10-12T23:02:18.095Z] 23:02:18     INFO - found chrome://browser/skin/preferences/preferences.css
[task 2018-10-12T23:02:18.098Z] 23:02:18     INFO - found chrome://global/skin/in-content/common.css
[task 2018-10-12T23:02:18.100Z] 23:02:18     INFO - found chrome://browser/skin/preferences/in-content/preferences.css
[task 2018-10-12T23:02:18.103Z] 23:02:18     INFO - found chrome://browser/skin/preferences/in-content/dialog.css
[task 2018-10-12T23:02:18.105Z] 23:02:18     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | All expectedStyleSheetURLs should have been found - 
[task 2018-10-12T23:02:18.292Z] 23:02:18     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_connection_dnsoverhttps.js | connection window opened - 
[task 2018-10-12T23:02:18.294Z] 23:02:18     INFO - 828: testWithProperties: connections dialog now open
[task 2018-10-12T23:02:18.296Z] 23:02:18     INFO - Not taking screenshot here: see the one that was previously logged
Flags: needinfo?(ewright)
Erica sent me a DMG to test locally, and everything looks good to me.
QA=R+

Only observations I had were:

0) Doesn't match the UX mocks attached to this bug, but the new design is an improvement.

1) Would be nice to add some future foot-gun protection around custom URLs, since it will happily let me enter a custom URL of "peter is an idiot". Not sure if it's as "easy" as RegExp checking the domain starts with /^https?:\/\// or if that's too naive of a restriction (and we want to support other protocols). But I can see somebody forgetting an "https://" and things possibly just breaking unexpectedly and difficult to diagnose.

Looking in about:config for `trr`, I see two keys w/ my custom bogus “peter is an idiot” domain:
`network.trr.custom_uri` and `network.trr.uri`

`network.trr.mode` seems to be non-default at `2` as well, which is probably expected since it’s a custom URL.
Attachment #9013465 - Attachment description: Bug 1495583 - Add a button to restore the default value for network.trr.uri preference. r?sfoster,flod → Bug 1495583 - Add a button to restore the default value for network.trr.uri preference. r?jaws,flod
Assignee

Comment 9

9 months ago
New patch with test changes has been posted. This will be paused for a bit due to string changes blocking the uplift.
Assignee

Updated

9 months ago
Blocks: 1498758
Assignee

Updated

9 months ago
Flags: needinfo?(ewright)
Assignee

Comment 10

8 months ago
This adds a button to the preferences UI to restore the default value for network.trr.uri preference.

Comment 11

8 months ago
Pushed by ewright@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8c6db36f8498
Add a button to restore the default value for network.trr.uri preference. r=flod,jaws

Comment 12

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8c6db36f8498
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Flags: qe-verify+

I have managed to reproduce the issue on Fx64.0a1 buildID: 20181001220118.
The issue is verified fixed using Fx65.0b12 and Fx66.0a1 on macOS 10.10, windows 10 x64 and ubuntu 16.04. There is a radio box option containing the default DNS value that can be chosen in the preferences/network settings section.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.