Bug 969282 - don't copy http proxy values to socks proxy now that we use it for websocket connections, r=mixedpuppy,mkaply,fluent-reviewers,flod
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 02 Jan 2020 12:51:40 +0000
changeset 508594 330661a663e8619e5ed63dc0fac751cb7f81baba
parent 508593 7d46024989e141c089cb2844747aecbf9574af29
child 508595 1a45094b7aa14dfb9c420d4645a0c284699fd94f
push id104101
push usergijskruitbosch@gmail.com
push dateThu, 02 Jan 2020 13:30:36 +0000
treeherderautoland@330661a663e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy, mkaply, fluent-reviewers, flod
bugs969282, 1577862, 1601871
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 969282 - don't copy http proxy values to socks proxy now that we use it for websocket connections, r=mixedpuppy,mkaply,fluent-reviewers,flod I also took the opportunity to fix the SSL proxy label - we shouldn't label things "SSL" anymore in 2019, unless they really are. All the comments in bug 969282, bug 1577862 and bug 1601871 use 'HTTPS'. Differential Revision: https://phabricator.services.mozilla.com/D58120
browser/components/BrowserGlue.jsm
browser/components/preferences/connection.js
browser/components/preferences/connection.xhtml
browser/components/preferences/in-content/main.inc.xhtml
browser/components/preferences/in-content/tests/browser_proxy_backup.js
browser/locales/en-US/browser/preferences/connection.ftl
browser/themes/shared/incontentprefs/preferences.inc.css
toolkit/components/extensions/parent/ext-proxy.js
toolkit/components/extensions/test/xpcshell/test_ext_proxy_config.js
--- a/browser/components/BrowserGlue.jsm
+++ b/browser/components/BrowserGlue.jsm
@@ -2790,17 +2790,17 @@ BrowserGlue.prototype = {
       );
     });
   },
 
   // eslint-disable-next-line complexity
   _migrateUI: function BG__migrateUI() {
     // Use an increasing number to keep track of the current migration state.
     // Completely unrelated to the current Firefox release number.
-    const UI_VERSION = 90;
+    const UI_VERSION = 91;
     const BROWSER_DOCURL = AppConstants.BROWSER_CHROME_URL;
 
     if (!Services.prefs.prefHasUserValue("browser.migration.version")) {
       // This is a new profile, nothing to migrate.
       Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
       this._isNewProfile = true;
       return;
     }
@@ -3255,16 +3255,39 @@ BrowserGlue.prototype = {
         "chrome://browser/content/places/places.xhtml"
       );
       this._migrateXULStoreForDocument(
         "chrome://browser/content/places/bookmarksSidebar.xul",
         "chrome://browser/content/places/bookmarksSidebar.xhtml"
       );
     }
 
+    // Clear socks proxy values if they were shared from http, to prevent
+    // websocket breakage after bug 1577862 (see bug 969282).
+    if (
+      currentUIVersion < 91 &&
+      Services.prefs.getBoolPref("network.proxy.share_proxy_settings", false) &&
+      Services.prefs.getIntPref("network.proxy.type", 0) == 1
+    ) {
+      let httpProxy = Services.prefs.getCharPref("network.proxy.http", "");
+      let httpPort = Services.prefs.getIntPref("network.proxy.http_port", 0);
+      let socksProxy = Services.prefs.getCharPref("network.proxy.socks", "");
+      let socksPort = Services.prefs.getIntPref("network.proxy.socks_port", 0);
+      if (httpProxy && httpProxy == socksProxy && httpPort == socksPort) {
+        Services.prefs.setCharPref(
+          "network.proxy.socks",
+          Services.prefs.getCharPref("network.proxy.backup.socks", "")
+        );
+        Services.prefs.setIntPref(
+          "network.proxy.socks_port",
+          Services.prefs.getIntPref("network.proxy.backup.socks_port", 0)
+        );
+      }
+    }
+
     // Update the migration version.
     Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
   },
 
   _checkForDefaultBrowser() {
     // Perform default browser checking.
     if (!ShellService) {
       return;
--- a/browser/components/preferences/connection.js
+++ b/browser/components/preferences/connection.js
@@ -132,33 +132,33 @@ var gConnectionsDialog = {
 
     // If the port is 0 and the proxy server is specified, focus on the port and cancel submission.
     for (let prefName of ["http", "ssl", "ftp", "socks"]) {
       let proxyPortPref = Preferences.get(
         "network.proxy." + prefName + "_port"
       );
       let proxyPref = Preferences.get("network.proxy." + prefName);
       // Only worry about ports which are currently active. If the share option is on, then ignore
-      // all ports except the HTTP port
+      // all ports except the HTTP and SOCKS port
       if (
         proxyPref.value != "" &&
         proxyPortPref.value == 0 &&
-        (prefName == "http" || !shareProxiesPref.value)
+        (prefName == "http" || prefName == "socks" || !shareProxiesPref.value)
       ) {
         document
           .getElementById("networkProxy" + prefName.toUpperCase() + "_Port")
           .focus();
         event.preventDefault();
         return;
       }
     }
 
     // In the case of a shared proxy preference, backup the current values and update with the HTTP value
     if (shareProxiesPref.value) {
-      var proxyPrefs = ["ssl", "ftp", "socks"];
+      var proxyPrefs = ["ssl", "ftp"];
       for (var i = 0; i < proxyPrefs.length; ++i) {
         var proxyServerURLPref = Preferences.get(
           "network.proxy." + proxyPrefs[i]
         );
         var proxyPortPref = Preferences.get(
           "network.proxy." + proxyPrefs[i] + "_port"
         );
         var backupServerURLPref = Preferences.get(
@@ -265,17 +265,17 @@ var gConnectionsDialog = {
       var proxyServerURLPref = Preferences.get(
         "network.proxy." + proxyPrefs[i]
       );
       var proxyPortPref = Preferences.get(
         "network.proxy." + proxyPrefs[i] + "_port"
       );
 
       // Restore previous per-proxy custom settings, if present.
-      if (!shareProxiesPref.value) {
+      if (proxyPrefs[i] != "socks" && !shareProxiesPref.value) {
         var backupServerURLPref = Preferences.get(
           "network.proxy.backup." + proxyPrefs[i]
         );
         var backupPortPref = Preferences.get(
           "network.proxy.backup." + proxyPrefs[i] + "_port"
         );
         if (backupServerURLPref.hasUserValue) {
           proxyServerURLPref.value = backupServerURLPref.value;
@@ -296,24 +296,26 @@ var gConnectionsDialog = {
     var socksVersionPref = Preferences.get("network.proxy.socks_version");
     socksVersionPref.disabled =
       proxyTypePref.value != 1 || shareProxiesPref.value;
     this.updateDNSPref();
     return undefined;
   },
 
   readProxyProtocolPref(aProtocol, aIsPort) {
-    var shareProxiesPref = Preferences.get(
-      "network.proxy.share_proxy_settings"
-    );
-    if (shareProxiesPref.value) {
-      var pref = Preferences.get(
-        "network.proxy.http" + (aIsPort ? "_port" : "")
+    if (aProtocol != "socks") {
+      var shareProxiesPref = Preferences.get(
+        "network.proxy.share_proxy_settings"
       );
-      return pref.value;
+      if (shareProxiesPref.value) {
+        var pref = Preferences.get(
+          "network.proxy.http" + (aIsPort ? "_port" : "")
+        );
+        return pref.value;
+      }
     }
 
     var backupPref = Preferences.get(
       "network.proxy.backup." + aProtocol + (aIsPort ? "_port" : "")
     );
     return backupPref.hasUserValue ? backupPref.value : undefined;
   },
 
--- a/browser/components/preferences/connection.xhtml
+++ b/browser/components/preferences/connection.xhtml
@@ -20,30 +20,16 @@
 
   <!-- Used for extension-controlled lockdown message -->
   <linkset>
     <html:link rel="localization" href="browser/preferences/connection.ftl"/>
     <html:link rel="localization" href="browser/preferences/preferences.ftl"/>
     <html:link rel="localization" href="branding/brand.ftl"/>
   </linkset>
 
-  <html:style>
-    #proxy-grid,
-    #dnsOverHttps-grid {
-      display: grid;
-      grid-template-columns: auto 1fr;
-      align-items: center;
-    }
-
-    #dnsOverHttps-grid.custom-container-hidden #networkCustomDnsOverHttpsInputLabelContainer,
-    #dnsOverHttps-grid.custom-container-hidden #networkCustomDnsOverHttpsInput {
-      display: none;
-    }
-  </html:style>
-
   <script src="chrome://browser/content/utilityOverlay.js"/>
   <script src="chrome://global/content/preferencesBindings.js"/>
   <script src="chrome://browser/content/preferences/in-content/extensionControlled.js"/>
 
   <keyset>
     <key data-l10n-id="connection-close-key" modifiers="accel" oncommand="Preferences.close(event)"/>
   </keyset>
 
@@ -73,37 +59,38 @@
           <html:input id="networkProxyHTTP" type="text" style="-moz-box-flex: 1;"
                       preference="network.proxy.http"/>
           <label data-l10n-id="connection-proxy-http-port" control="networkProxyHTTP_Port" />
           <html:input id="networkProxyHTTP_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535"
                       preference="network.proxy.http_port"/>
         </hbox>
         <hbox/>
         <hbox>
-          <checkbox id="shareAllProxies" data-l10n-id="connection-proxy-http-share"
+          <checkbox id="shareAllProxies" data-l10n-id="connection-proxy-http-sharing"
                     preference="network.proxy.share_proxy_settings"/>
         </hbox>
         <hbox pack="end">
-          <label data-l10n-id="connection-proxy-ssl" control="networkProxySSL"/>
+          <label data-l10n-id="connection-proxy-https" control="networkProxySSL"/>
         </hbox>
         <hbox align="center">
           <html:input id="networkProxySSL" type="text" style="-moz-box-flex: 1;" preference="network.proxy.ssl"/>
           <label data-l10n-id="connection-proxy-ssl-port" control="networkProxySSL_Port" />
           <html:input id="networkProxySSL_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
                       preference="network.proxy.ssl_port"/>
         </hbox>
         <hbox pack="end">
           <label data-l10n-id="connection-proxy-ftp" control="networkProxyFTP"/>
         </hbox>
         <hbox align="center">
           <html:input id="networkProxyFTP" type="text" style="-moz-box-flex: 1;" preference="network.proxy.ftp"/>
           <label data-l10n-id="connection-proxy-ftp-port" control="networkProxyFTP_Port"/>
           <html:input id="networkProxyFTP_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
                       preference="network.proxy.ftp_port"/>
         </hbox>
+        <separator class="thin"/>
         <hbox pack="end">
           <label data-l10n-id="connection-proxy-socks" control="networkProxySOCKS"/>
         </hbox>
         <hbox align="center">
           <html:input id="networkProxySOCKS" type="text" style="-moz-box-flex: 1;" preference="network.proxy.socks"/>
           <label data-l10n-id="connection-proxy-socks-port" control="networkProxySOCKS_Port"/>
           <html:input id="networkProxySOCKS_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
                       preference="network.proxy.socks_port"/>
--- a/browser/components/preferences/in-content/main.inc.xhtml
+++ b/browser/components/preferences/in-content/main.inc.xhtml
@@ -706,25 +706,25 @@
               searchkeywords="doh trr"
               search-l10n-ids="
                 connection-window.title,
                 connection-proxy-option-no.label,
                 connection-proxy-option-auto.label,
                 connection-proxy-option-system.label,
                 connection-proxy-option-manual.label,
                 connection-proxy-http,
-                connection-proxy-ssl,
+                connection-proxy-https,
                 connection-proxy-ftp,
                 connection-proxy-http-port,
                 connection-proxy-socks,
                 connection-proxy-socks4,
                 connection-proxy-socks5,
                 connection-proxy-noproxy,
                 connection-proxy-noproxy-desc,
-                connection-proxy-http-share.label,
+                connection-proxy-http-sharing.label,
                 connection-proxy-autotype.label,
                 connection-proxy-reload.label,
                 connection-proxy-autologin.label,
                 connection-proxy-socks-remote-dns.label,
                 connection-dns-over-https.label,
                 connection-dns-over-https-url-custom.label,
             " />
     </hbox>
--- a/browser/components/preferences/in-content/tests/browser_proxy_backup.js
+++ b/browser/components/preferences/in-content/tests/browser_proxy_backup.js
@@ -25,25 +25,25 @@ function test() {
       Services.prefs.clearUserPref(
         "network.proxy.backup." + proxyType + "_port"
       );
     }
   });
 
   let connectionURL = "chrome://browser/content/preferences/connection.xhtml";
 
-  // Set a shared proxy and a SOCKS backup
+  // Set a shared proxy and an SSL backup
   Services.prefs.setIntPref("network.proxy.type", 1);
   Services.prefs.setBoolPref("network.proxy.share_proxy_settings", true);
   Services.prefs.setCharPref("network.proxy.http", "example.com");
   Services.prefs.setIntPref("network.proxy.http_port", 1200);
-  Services.prefs.setCharPref("network.proxy.socks", "example.com");
-  Services.prefs.setIntPref("network.proxy.socks_port", 1200);
-  Services.prefs.setCharPref("network.proxy.backup.socks", "127.0.0.1");
-  Services.prefs.setIntPref("network.proxy.backup.socks_port", 9050);
+  Services.prefs.setCharPref("network.proxy.ssl", "example.com");
+  Services.prefs.setIntPref("network.proxy.ssl_port", 1200);
+  Services.prefs.setCharPref("network.proxy.backup.ssl", "127.0.0.1");
+  Services.prefs.setIntPref("network.proxy.backup.ssl_port", 9050);
 
   /*
   The connection dialog alone won't save onaccept since it uses type="child",
   so it has to be opened as a sub dialog of the main pref tab.
   Open the main tab here.
   */
   open_preferences(async function tabOpened(aContentWindow) {
     is(
@@ -59,24 +59,24 @@ function test() {
     );
 
     ok(dialog, "connection window opened");
     dialogElement.acceptDialog();
 
     let dialogClosingEvent = await dialogClosingPromise;
     ok(dialogClosingEvent, "connection window closed");
 
-    // The SOCKS backup should not be replaced by the shared value
+    // The SSL backup should not be replaced by the shared value
     is(
-      Services.prefs.getCharPref("network.proxy.backup.socks"),
+      Services.prefs.getCharPref("network.proxy.backup.ssl"),
       "127.0.0.1",
       "Shared proxy backup shouldn't be replaced"
     );
     is(
-      Services.prefs.getIntPref("network.proxy.backup.socks_port"),
+      Services.prefs.getIntPref("network.proxy.backup.ssl_port"),
       9050,
       "Shared proxy port backup shouldn't be replaced"
     );
 
     gBrowser.removeCurrentTab();
     finish();
   });
 }
--- a/browser/locales/en-US/browser/preferences/connection.ftl
+++ b/browser/locales/en-US/browser/preferences/connection.ftl
@@ -30,22 +30,22 @@ connection-proxy-option-auto =
 connection-proxy-option-manual =
     .label = Manual proxy configuration
     .accesskey = M
 
 connection-proxy-http = HTTP Proxy
     .accesskey = x
 connection-proxy-http-port = Port
     .accesskey = P
-connection-proxy-http-share =
-    .label = Use this proxy server for all protocols
+connection-proxy-http-sharing =
+    .label = Also use this proxy for FTP and HTTPS
     .accesskey = s
 
-connection-proxy-ssl = SSL Proxy
-    .accesskey = L
+connection-proxy-https = HTTPS Proxy
+    .accesskey = H
 connection-proxy-ssl-port = Port
     .accesskey = o
 
 connection-proxy-ftp = FTP Proxy
     .accesskey = F
 connection-proxy-ftp-port = Port
     .accesskey = r
 
--- a/browser/themes/shared/incontentprefs/preferences.inc.css
+++ b/browser/themes/shared/incontentprefs/preferences.inc.css
@@ -1038,8 +1038,25 @@ menulist[indicator=true] > menupopup men
 richlistitem .text-link {
   color: inherit;
   text-decoration: underline;
 }
 
 richlistitem .text-link:hover {
   color: inherit;
 }
+
+#proxy-grid,
+#dnsOverHttps-grid {
+  display: grid;
+  grid-template-columns: auto 1fr;
+  align-items: center;
+}
+
+#proxy-grid > .thin {
+  grid-column-end: 3;
+  height: 20px;
+}
+
+#dnsOverHttps-grid.custom-container-hidden #networkCustomDnsOverHttpsInputLabelContainer,
+#dnsOverHttps-grid.custom-container-hidden #networkCustomDnsOverHttpsInput {
+  display: none;
+}
--- a/toolkit/components/extensions/parent/ext-proxy.js
+++ b/toolkit/components/extensions/parent/ext-proxy.js
@@ -254,17 +254,17 @@ this.proxy = class extends ExtensionAPI 
                   `${value.proxyType} is not a valid value for proxyType.`
                 );
               }
 
               if (value.httpProxyAll) {
                 // Match what about:preferences does with proxy settings
                 // since the proxy service does not check the value
                 // of share_proxy_settings.
-                for (let prop of ["ftp", "ssl", "socks"]) {
+                for (let prop of ["ftp", "ssl"]) {
                   value[prop] = value.http;
                 }
               }
 
               for (let prop of ["http", "ftp", "ssl", "socks"]) {
                 let host = value[prop];
                 if (host) {
                   try {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_proxy_config.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_proxy_config.js
@@ -215,26 +215,24 @@ add_task(async function test_browser_set
     {
       "network.proxy.type": proxySvc.PROXYCONFIG_MANUAL,
       "network.proxy.http": "www.mozilla.org",
       "network.proxy.http_port": 8080,
       "network.proxy.ftp": "www.mozilla.org",
       "network.proxy.ftp_port": 8080,
       "network.proxy.ssl": "www.mozilla.org",
       "network.proxy.ssl_port": 8080,
-      "network.proxy.socks": "www.mozilla.org",
-      "network.proxy.socks_port": 8080,
       "network.proxy.share_proxy_settings": true,
     },
     {
       proxyType: "manual",
       http: "www.mozilla.org:8080",
       ftp: "www.mozilla.org:8080",
       ssl: "www.mozilla.org:8080",
-      socks: "www.mozilla.org:8080",
+      socks: "",
       httpProxyAll: true,
     }
   );
 
   await testProxy(
     {
       proxyType: "manual",
       http: "www.mozilla.org:8080",