1495583 - add restore default value to DNS-over-HTTPS preferences

Status: VERIFIED FIXED

(Firefox :: Settings UI, enhancement, P1)

Description

[Erica Wright [:ewright]]

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.

Comment 1

[Erica Wright [:ewright]]

Attachment: restore-default-url-mock.png

Comment 2

[Erica Wright [:ewright]]

Attachment: restore-default-url-disabled-mock.png

Comment 3

[Erica Wright [:ewright]]

Attachment: Bug 1495583 - Add a button to restore the default value for network.trr.uri preference. r?jaws,flod

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

Comment 4

[Erica Wright [:ewright]]

: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.

Comment 5

[Jason Duell]

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74a98c04cd54
Add a button to restore the default value for network.trr.uri preference. r=flod,jaws

Comment 7

[Cosmin Sabou [:CosminS]]

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

Comment 8

[Peter deHaan [:pdehaan]]

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.

Comment 9

[Erica Wright [:ewright]]

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

Comment 10

[Erica Wright [:ewright]]

Attachment: Bug 1495583 - Add a button to restore the default value for network.trr.uri preference.

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
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

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/8c6db36f8498

Comment 13

[Cristian Baica [:cbaica], Release Desktop QA]

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.