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 --HG-- extra : moz-landing-system : lando
2019-12-30 22:20:02 +00:00 · 2019-12-30 22:20:02 +00:00 · 16ba4e5886
--- a/browser/components/BrowserGlue.jsm
+++ b/browser/components/BrowserGlue.jsm
@ -2795,7 +2795,7 @@ BrowserGlue.prototype = {
  _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")) {
@ -3260,6 +3260,29 @@ BrowserGlue.prototype = {
      );
    }

+    // 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);
  },
--- a/browser/components/preferences/connection.js
+++ b/browser/components/preferences/connection.js
@ -137,11 +137,11 @@ var gConnectionsDialog = {
      );
      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")
@ -153,7 +153,7 @@ var gConnectionsDialog = {

    // 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]
@ -270,7 +270,7 @@ var gConnectionsDialog = {
      );

      // 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]
        );
@ -301,14 +301,16 @@ var gConnectionsDialog = {
  },

  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(
--- a/browser/components/preferences/connection.xhtml
+++ b/browser/components/preferences/connection.xhtml
@ -25,20 +25,6 @@
    <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"/>
@ -78,11 +64,11 @@
        </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"/>
@ -99,6 +85,7 @@
          <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>
--- a/browser/components/preferences/in-content/main.inc.xhtml
+++ b/browser/components/preferences/in-content/main.inc.xhtml
@ -711,7 +711,7 @@
                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,
@ -719,7 +719,7 @@
                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,
--- a/browser/components/preferences/in-content/tests/browser_proxy_backup.js
+++ b/browser/components/preferences/in-content/tests/browser_proxy_backup.js
@ -30,15 +30,15 @@ function test() {

  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",
@ -64,14 +64,14 @@ function test() {
    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"
    );
--- a/browser/locales/en-US/browser/preferences/connection.ftl
+++ b/browser/locales/en-US/browser/preferences/connection.ftl
@ -35,12 +35,12 @@ 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

--- a/browser/themes/shared/incontentprefs/preferences.inc.css
+++ b/browser/themes/shared/incontentprefs/preferences.inc.css
@ -1043,3 +1043,20 @@ richlistitem .text-link {
 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
@ -259,7 +259,7 @@ this.proxy = class extends ExtensionAPI {
                // 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;
                }
              }
--- a/toolkit/components/extensions/test/xpcshell/test_ext_proxy_config.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_proxy_config.js
@ -220,8 +220,6 @@ add_task(async function test_browser_settings() {
      "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,
    },
    {