Bug 1862404 - Scheme not properly shown on overflow by RTL domains. r=dao

Differential Revision: https://phabricator.services.mozilla.com/D210687

browser/components/urlbar/UrlbarPrefs.sys.mjs

@@ -518,6 +518,7 @@ const PREF_OTHER_DEFAULTS = new Map([
   ["browser.search.suggest.enabled.private", false],
   ["browser.search.widget.inNavBar", false],
   ["keyword.enabled", true],
+  ["security.insecure_connection_text.enabled", false],
   ["ui.popup.disable_autohide", false],
 ]);
 

browser/components/urlbar/UrlbarValueFormatter.sys.mjs

@@ -75,7 +75,7 @@ export class UrlbarValueFormatter {
     this.urlbarInput.removeAttribute("domaindir");
     this.scheme.value = "";
 
-    if (!this.inputField.value) {
+    if (!this.urlbarInput.value) {
       return;
     }
 
@@ -133,7 +133,7 @@ export class UrlbarValueFormatter {
       return null;
     }
 
-    let inputValue = this.inputField.value;
+    let inputValue = this.urlbarInput.value;
     // getFixupURIInfo logs an error if the URL is empty. Avoid that by
     // returning early.
     if (!inputValue) {
@@ -183,20 +183,28 @@ export class UrlbarValueFormatter {
       return null;
     }
 
-    // If we trimmed off the http scheme, ensure we stick it back on before
+    // We must ensure the protocol is present in the parsed string, so we don't
-    // trying to figure out what domain we're accessing, so we don't get
+    // get confused by user:pass@host. It may not have been present originally,
-    // confused by user:pass@host http URLs. We later use
+    // or it may have been trimmed. We later use trimmedLength to ensure we
-    // trimmedLength to ensure we don't count the length of a trimmed protocol
+    // don't count the length of a trimmed protocol when determining which parts
-    // when determining which parts of the URL to highlight as "preDomain".
+    // of the input value to de-emphasize as `preDomain`.
     let url = inputValue;
     let trimmedLength = 0;
     let trimmedProtocol = lazy.BrowserUIUtils.trimURLProtocol;
     if (
-      uriInfo.fixedURI.spec.startsWith(trimmedProtocol) &&
+      this.urlbarInput.untrimmedValue.startsWith(trimmedProtocol) &&
       !inputValue.startsWith(trimmedProtocol)
     ) {
+      // The protocol has been trimmed, so we add it back.
       url = trimmedProtocol + inputValue;
       trimmedLength = trimmedProtocol.length;
+    } else if (uriInfo.wasSchemelessInput) {
+      // The original string didn't have a protocol, but it was identified as
+      // a URL. It's not important which scheme we use for parsing, so we'll
+      // just copy URIFixup.
+      let scheme = uriInfo.fixedURI.scheme + "://";
+      url = scheme + url;
+      trimmedLength = scheme.length;
     }
 
     // This RegExp is not a perfect match, and for specially crafted URLs it may
@@ -278,14 +286,28 @@ export class UrlbarValueFormatter {
     let { domain, origin, preDomain, schemeWSlashes, trimmedLength, url } =
       urlMetaData;
 
-    let schemeStripped =
+    let isMixedContent =
-      lazy.UrlbarPrefs.get("trimURLs") &&
+      schemeWSlashes == "https://" &&
-      schemeWSlashes == lazy.BrowserUIUtils.trimURLProtocol;
+      this.urlbarInput.value.startsWith("https://") &&
+      this.urlbarInput.getAttribute("pageproxystate") == "valid" &&
+      this.window.gBrowser.securityUI.state &
+        Ci.nsIWebProgressListener.STATE_LOADED_MIXED_ACTIVE_CONTENT;
+    let isUnformattedMixedContent =
+      isMixedContent && !lazy.UrlbarPrefs.get("formatting.enabled");
 
-    // When the scheme is not stripped, add the scheme size as a property.
+    // When RTL domains cause the address bar to overflow to the left, the
-    // The scheme-size is used to prevent the scheme from being hidden, when
+    // protocol may get hidden, if it was not trimmed. We then set the
-    // RTL domains overflow to the left.
+    // `--urlbar-scheme-size` property to show the protocol in a floating box.
-    if (!schemeStripped) {
+    // We don't show the floating protocol box if:
+    //  - The protocol was trimmed.
+    //  - We're in mixed mode, but formatting is disabled. The not struck out
+    //    box may make the user think the connection is fully secure.
+    //  - The insecure label is active. The label is a sufficient indicator.
+    if (
+      this.urlbarInput.value.startsWith(schemeWSlashes) &&
+      !isUnformattedMixedContent &&
+      !lazy.UrlbarPrefs.get("security.insecure_connection_text.enabled")
+    ) {
       this.scheme.value = schemeWSlashes;
       this.inputField.style.setProperty(
         "--urlbar-scheme-size",
@@ -308,13 +330,7 @@ export class UrlbarValueFormatter {
 
     // Strike out the "https" part if mixed active content is loaded and https
     // is not trimmed.
-    if (
+    if (isMixedContent) {
-      !schemeStripped &&
-      this.urlbarInput.getAttribute("pageproxystate") == "valid" &&
-      url.startsWith("https:") &&
-      this.window.gBrowser.securityUI.state &
-        Ci.nsIWebProgressListener.STATE_LOADED_MIXED_ACTIVE_CONTENT
-    ) {
       let range = this.document.createRange();
       range.setStart(textNode, 0);
       range.setEnd(textNode, 5);

browser/components/urlbar/tests/browser/browser_UrlbarInput_overflow.js

@@ -3,7 +3,7 @@
  */
 
 async function testVal(aExpected, overflowSide = null) {
-  info(`Testing ${aExpected}`);
+  info(`Testing ${aExpected} with overflow ${overflowSide}`);
   try {
     gURLBar.setURI(makeURI(aExpected));
   } catch (ex) {
@@ -45,10 +45,14 @@ async function testVal(aExpected, overflowSide = null) {
   );
 
   let scheme = aExpected.match(/^([a-z]+:\/{0,2})/)?.[1] || "";
-  // We strip http, so we should not show the scheme for it.
+  // If we strip the protocol, or insecure label is enabled, we should not show
+  // the scheme for it.
   if (
-    scheme == "http://" &&
+    !gURLBar.value.startsWith(scheme) ||
-    Services.prefs.getBoolPref("browser.urlbar.trimURLs", true)
+    Services.prefs.getBoolPref(
+      "security.insecure_connection_text.enabled",
+      false
+    )
   ) {
     scheme = "";
   }
@@ -56,7 +60,7 @@ async function testVal(aExpected, overflowSide = null) {
   Assert.equal(
     gURLBar.valueFormatter.scheme.value,
     scheme,
-    "Check the scheme value"
+    "Check the scheme value after blur"
   );
   let isOverflowed =
     gURLBar.inputField.scrollWidth > gURLBar.inputField.clientWidth;
@@ -95,9 +99,6 @@ async function testVal(aExpected, overflowSide = null) {
 }
 
 add_task(async function () {
-  await SpecialPowers.pushPrefEnv({
-    set: [["browser.urlbar.trimHttps", false]],
-  });
   // We use a new tab for the test to be sure all the tab switching and loading
   // is complete before starting, otherwise onLocationChange for this tab could
   // override the value we set with an empty value.
@@ -132,7 +133,7 @@ add_task(async function () {
   await testVal(`http://${rtlDomain}:8888/${lotsOfSpaces}/test/`, "left");
   await testVal(`http://[::1]/${rtlChar}/${lotsOfSpaces}/test/`, "right");
 
-  info("Test with formatting disabled");
+  info("Test with formatting and trimURLs disabled");
   await SpecialPowers.pushPrefEnv({
     set: [
       ["browser.urlbar.formatting.enabled", false],
@@ -143,8 +144,6 @@ add_task(async function () {
   await testVal(`https://mozilla.org/`);
   await testVal(`https://${rtlDomain}/${lotsOfSpaces}/test/`, "left");
   await testVal(`https://mozilla.org/${lotsOfSpaces}/test/`, "right");
-
-  info("Test with trimURLs disabled");
   await testVal(`http://${rtlDomain}/${lotsOfSpaces}/test/`, "left");
 
   await SpecialPowers.popPrefEnv();

browser/components/urlbar/tests/browser/browser_urlbar_selection.js

@@ -186,7 +186,7 @@ add_task(async function rightClickSelectsAll() {
 
   gURLBar.querySelector("moz-input-box").menupopup.hidePopup();
   gURLBar.blur();
-  checkPrimarySelection(gURLBar._untrimmedValue);
+  checkPrimarySelection(gURLBar.untrimmedValue);
   await SpecialPowers.popPrefEnv();
 });