Bug 1263698 - Correctly place the em dash separator between title and URL in awesomebar popup in RTL. r?mak draft
authorDrew Willcoxon <adw@mozilla.com>
Mon, 11 Apr 2016 12:11:48 -0700
changeset 349513 3d37bcc57157089d73f880e6a1e2d1c1b09d681a
parent 348897 34293bb847d1e28198d596a87ea4baf96bfb0286
child 518122 e9aad9455ffaeb6d9c49dcffb8a5b6d896b407a6
push id15111
push userdwillcoxon@mozilla.com
push dateMon, 11 Apr 2016 19:12:06 +0000
reviewersmak
bugs1263698
milestone48.0a1
Bug 1263698 - Correctly place the em dash separator between title and URL in awesomebar popup in RTL. r?mak MozReview-Commit-ID: HpLCfE42KEF
browser/base/content/test/general/browser_search_favicon.js
browser/base/content/test/general/browser_urlbarDecode.js
browser/themes/linux/browser.css
browser/themes/osx/browser.css
browser/themes/windows/browser.css
toolkit/content/autocomplete.css
toolkit/content/tests/chrome/test_autocomplete_emphasis.xul
toolkit/content/widgets/autocomplete.xml
toolkit/themes/linux/global/autocomplete.css
toolkit/themes/osx/global/autocomplete.css
toolkit/themes/windows/global/autocomplete.css
--- a/browser/base/content/test/general/browser_search_favicon.js
+++ b/browser/base/content/test/general/browser_search_favicon.js
@@ -48,13 +48,12 @@ add_task(function*() {
 
   let urlHbox = result._urlText.parentNode.parentNode;
   ok(urlHbox.classList.contains("ac-url"), "URL hbox sanity check");
   is_element_hidden(urlHbox, "URL element should be hidden");
 
   let actionHbox = result._actionText.parentNode.parentNode;
   ok(actionHbox.classList.contains("ac-action"), "Action hbox sanity check");
   is_element_hidden(actionHbox, "Action element should be hidden because it is not selected");
-  // \u2014 == em dash
-  is(result._actionText.textContent, "\u2014Search with SearchEngine", "Action text should be as expected");
+  is(result._actionText.textContent, "Search with SearchEngine", "Action text should be as expected");
 
   gBrowser.removeCurrentTab();
 });
--- a/browser/base/content/test/general/browser_urlbarDecode.js
+++ b/browser/base/content/test/general/browser_urlbarDecode.js
@@ -89,11 +89,10 @@ function* checkInput(inputStr) {
 
   let itemTypeStr = item.getAttribute("type");
   let itemTypes = itemTypeStr.split(" ").sort();
   Assert.equal(itemTypes.toString(),
                ["action", "heuristic", "visiturl"].toString(),
                "type");
 
   Assert.equal(item._titleText.textContent, inputStr.replace("\\","/"), "Visible title");
-  // \u2014 == em dash
-  Assert.equal(item._actionText.textContent, "\u2014Visit", "Visible action");
+  Assert.equal(item._actionText.textContent, "Visit", "Visible action");
 }
--- a/browser/themes/linux/browser.css
+++ b/browser/themes/linux/browser.css
@@ -1128,23 +1128,25 @@ notification[value="translation"] menuli
 html|span.ac-tag {
   background-color: MenuText;
   color: Menu;
   border-radius: 2px;
   border: 1px solid transparent;
   padding: 0 1px;
 }
 
+.ac-separator,
 .ac-url,
 .ac-action {
   font-size: 12px;
   color: -moz-nativehyperlinktext;
 }
 
 .ac-title[selected=true],
+.ac-separator[selected],
 .ac-url[selected=true],
 .ac-action[selected=true] {
   color: inherit !important;
 }
 
 .ac-tags-text[selected] > html|span.ac-tag {
   background-color: HighlightText;
   color: Highlight;
--- a/browser/themes/osx/browser.css
+++ b/browser/themes/osx/browser.css
@@ -1756,42 +1756,45 @@ html|span.ac-tag {
   border: 1px solid transparent;
   padding: 0 1px;
 }
 
 html|span.ac-emphasize-text-tag {
   color: hsl(0, 0%, 50%);
 }
 
+.ac-separator,
+.ac-url,
+.ac-action {
+  font-size: 12px;
+}
+
+.ac-separator {
+  color: hsl(0, 0%, 50%);
+}
+
 .ac-url {
-  font-size: 12px;
   color: hsl(210, 77%, 47%);
 }
 
 html|span.ac-emphasize-text-url {
   color: hsl(210, 86%, 64%);
 }
 
 .ac-action {
-  font-size: 12px;
   color: hsl(178, 100%, 28%);
 }
 
-html|span.ac-title-urlaction-separator {
-  color: hsl(0, 0%, 50%);
-}
-
 .ac-title[selected],
+.ac-separator[selected],
 .ac-url[selected],
 .ac-action[selected],
 .ac-title-text[selected] > html|span.ac-emphasize-text,
 .ac-url-text[selected] > html|span.ac-emphasize-text,
-.ac-action-text[selected] > html|span.ac-emphasize-text,
-.ac-url-text[selected] > html|span.ac-title-urlaction-separator,
-.ac-action-text[selected] > html|span.ac-title-urlaction-separator {
+.ac-action-text[selected] > html|span.ac-emphasize-text {
   color: hsl(0, 0%, 100%);
 }
 
 .ac-tags-text[selected] > html|span.ac-tag {
   background-color: hsl(0, 0%, 100%);
   color: hsl(210, 80%, 40%);
 }
 
--- a/browser/themes/windows/browser.css
+++ b/browser/themes/windows/browser.css
@@ -1444,42 +1444,45 @@ html|span.ac-tag {
   border: 1px solid transparent;
   padding: 0 1px;
 }
 
 html|span.ac-emphasize-text-tag {
   color: hsl(0, 0%, 50%);
 }
 
+.ac-separator,
+.ac-url,
+.ac-action {
+  font-size: 12px;
+}
+
+.ac-separator {
+  color: hsl(0, 0%, 50%);
+}
+
 .ac-url {
-  font-size: 12px;
   color: hsl(210, 77%, 47%);
 }
 
 html|span.ac-emphasize-text-url {
   color: hsl(210, 86%, 64%);
 }
 
 .ac-action {
-  font-size: 12px;
   color: hsl(178, 100%, 28%);
 }
 
-html|span.ac-title-urlaction-separator {
-  color: hsl(0, 0%, 50%);
-}
-
 .ac-title[selected=true],
+.ac-separator[selected],
 .ac-url[selected=true],
 .ac-action[selected=true],
 .ac-title-text[selected=true] > html|span.ac-emphasize-text,
 .ac-url-text[selected=true] > html|span.ac-emphasize-text,
-.ac-action-text[selected=true] > html|span.ac-emphasize-text,
-.ac-url-text[selected=true] > html|span.ac-title-urlaction-separator,
-.ac-action-text[selected=true] > html|span.ac-title-urlaction-separator {
+.ac-action-text[selected=true] > html|span.ac-emphasize-text {
   color: hsl(0, 0%, 100%);
 }
 
 .ac-tags-text[selected] > html|span.ac-tag {
   background-color: hsl(0, 0%, 100%);
   color: hsl(210, 80%, 40%);
 }
 
@@ -1498,20 +1501,20 @@ html|span.ac-title-urlaction-separator {
   }
 
   html|span.ac-tag,
   html|span.ac-emphasize-text-tag {
     background-color: -moz-FieldText;
     color: -moz-Field;
   }
 
+  .ac-separator,
   .ac-url,
   .ac-action,
-  html|span.ac-emphasize-text-url,
-  html|span.ac-title-urlaction-separator {
+  html|span.ac-emphasize-text-url {
     color: -moz-nativehyperlinktext;
   }
 
   .ac-tags-text[selected] > html|span.ac-tag,
   .ac-tags-text[selected] > html|span.ac-tag > html|span.ac-emphasize-text-tag {
     background-color: HighlightText;
     color: Highlight;
   }
--- a/toolkit/content/autocomplete.css
+++ b/toolkit/content/autocomplete.css
@@ -25,11 +25,12 @@ richlistitem {
   text-overflow: ellipsis;
   white-space: nowrap;
 }
 
 .ac-tags[empty] {
   display: none;
 }
 
-.ac-action[actiontype=searchengine]:not([selected]) {
+.ac-action[actiontype=searchengine]:not([selected]),
+.ac-separator[actiontype=searchengine]:not([selected]) {
   display: none;
 }
--- a/toolkit/content/tests/chrome/test_autocomplete_emphasis.xul
+++ b/toolkit/content/tests/chrome/test_autocomplete_emphasis.xul
@@ -134,33 +134,20 @@ function nextTest() {
   synthesizeKey("VK_DOWN", {});
 }
 
 function checkSearchCompleted() {
   let autocomplete = $("richautocomplete");
   let result = autocomplete.popup.richlistbox.firstChild;
 
   for (let attribute of [result._titleText, result._urlText]) {
-
-    let numChildren = currentTest.emphasis.length;
-    let childNodeStart = 0;
-    if (attribute == result._urlText) {
-      // For the URL description, the first child is the em dash separator that
-      // visually separates the the title string from the URL string.
-      numChildren++;
-      childNodeStart = 1;
-      let node = attribute.childNodes[0];
-      ok(node.classList.contains("ac-title-urlaction-separator"),
-         "First child of URL text should be separator");
-    }
-
-    is(attribute.childNodes.length, numChildren, "The element should have the expected number of children.");
-
+    is(attribute.childNodes.length, currentTest.emphasis.length,
+       "The element should have the expected number of children.");
     for (let i = 0; i < currentTest.emphasis.length; i++) {
-      let node = attribute.childNodes[childNodeStart + i];
+      let node = attribute.childNodes[i];
       // Emphasized parts strictly alternate.
       if ((i % 2 == 0) == currentTest.emphasizeFirst) {
         // Check that this part is correctly emphasized.
         is(node.nodeName, "span", ". That child should be a span node");
         ok(node.classList.contains("ac-emphasize-text"), ". That child should be emphasized");
         is(node.textContent, currentTest.emphasis[i], ". That emphasis should be as expected.");
       } else {
         // Check that this part is _not_ emphasized.
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1354,16 +1354,22 @@ extends="chrome://global/content/binding
                 align="center"
                 xbl:inherits="selected">
         <xul:description class="ac-text-overflow-container">
           <xul:description anonid="tags-text"
                            class="ac-tags-text"
                            xbl:inherits="selected"/>
         </xul:description>
       </xul:hbox>
+      <xul:hbox anonid="separator"
+                class="ac-separator"
+                align="center"
+                xbl:inherits="selected,actiontype">
+        <xul:description class="ac-separator-text">—</xul:description>
+      </xul:hbox>
       <xul:hbox class="ac-url"
                 align="center"
                 xbl:inherits="selected,actiontype">
         <xul:description class="ac-text-overflow-container">
           <xul:description anonid="url-text"
                            class="ac-url-text"
                            xbl:inherits="selected"/>
         </xul:description>
@@ -1392,16 +1398,19 @@ extends="chrome://global/content/binding
             this, "anonid", "title-text"
           );
           this._tags = document.getAnonymousElementByAttribute(
             this, "anonid", "tags"
           );
           this._tagsText = document.getAnonymousElementByAttribute(
             this, "anonid", "tags-text"
           );
+          this._separator = document.getAnonymousElementByAttribute(
+            this, "anonid", "separator"
+          );
           this._urlText = document.getAnonymousElementByAttribute(
             this, "anonid", "url-text"
           );
           this._actionText = document.getAnonymousElementByAttribute(
             this, "anonid", "action-text"
           );
           this._adjustAcItem();
         ]]>
@@ -1555,27 +1564,16 @@ extends="chrome://global/content/binding
         <parameter name="aText"/>
         <parameter name="aNoEmphasis"/>
         <body>
           <![CDATA[
           // Get rid of all previous text
           while (aDescriptionElement.hasChildNodes())
             aDescriptionElement.removeChild(aDescriptionElement.firstChild);
 
-          // Add a separator to the front of the URL and action.
-          if (aText &&
-              (aDescriptionElement == this._urlText ||
-               aDescriptionElement == this._actionText)) {
-            let span =
-              document.createElementNS("http://www.w3.org/1999/xhtml", "span");
-            aDescriptionElement.appendChild(span);
-            span.className = "ac-title-urlaction-separator";
-            span.textContent = "—";
-          }
-
           // If aNoEmphasis is specified, don't add any emphasis
           if (aNoEmphasis) {
             aDescriptionElement.appendChild(document.createTextNode(aText));
             return;
           }
 
           // Get the indices that separate match and non-match text
           let search = this.getAttribute("text");
@@ -1999,19 +1997,21 @@ extends="chrome://global/content/binding
         </body>
       </method>
 
       <!-- This method truncates the displayed strings as necessary. -->
       <method name="_handleOverflow">
         <body><![CDATA[
           let titleRect = this._titleText.getBoundingClientRect();
           let tagsRect = this._tagsText.getBoundingClientRect();
+          let separatorRect = this._separator.getBoundingClientRect();
           let urlRect = this._urlText.getBoundingClientRect();
           let actionRect = this._actionText.getBoundingClientRect();
-          let urlActionWidth = Math.max(urlRect.width, actionRect.width);
+          let separatorURLActionWidth =
+            separatorRect.width + Math.max(urlRect.width, actionRect.width);
 
           // Total width for the title and URL/action is the width of the item
           // minus the start of the title text minus a little extra padding.
           // This extra padding amount is basically arbitrary but balances out
           // the listbox's padding on the left side.
           let extraPadding = 30;
           let itemWidth =
             this.parentNode.getBoundingClientRect().width -
@@ -2019,23 +2019,23 @@ extends="chrome://global/content/binding
             extraPadding;
 
           if (this._tags.hasAttribute("empty")) {
             // The tags box is not displayed in this case.
             tagsRect.width = 0;
           }
 
           let titleTagsWidth = titleRect.width + tagsRect.width;
-          if (titleTagsWidth + urlActionWidth > itemWidth) {
+          if (titleTagsWidth + separatorURLActionWidth > itemWidth) {
             // Title + tags + URL/action overflows the item width.
 
             // The percentage of the item width allocated to the title and tags.
             let titleTagsPct = 0.66;
 
-            let titleTagsAvailable = itemWidth - urlActionWidth;
+            let titleTagsAvailable = itemWidth - separatorURLActionWidth;
             let titleTagsMaxWidth = Math.max(
               titleTagsAvailable,
               itemWidth * titleTagsPct
             );
             if (titleTagsWidth > titleTagsMaxWidth) {
               // Title + tags overflows the max title + tags width.
 
               // The percentage of the title + tags width allocated to the
@@ -2050,21 +2050,21 @@ extends="chrome://global/content/binding
               let tagsAvailable = titleTagsMaxWidth - titleRect.width;
               let tagsMaxWidth = Math.max(
                 tagsAvailable,
                 titleTagsMaxWidth * (1 - titlePct)
               );
               this._titleText.style.maxWidth = titleMaxWidth + "px";
               this._tagsText.style.maxWidth = tagsMaxWidth + "px";
             }
-            let urlActionAvailable = itemWidth - titleTagsWidth;
             let urlActionMaxWidth = Math.max(
-              urlActionAvailable,
+              itemWidth - titleTagsWidth,
               itemWidth * (1 - titleTagsPct)
             );
+            urlActionMaxWidth -= separatorRect.width;
             this._urlText.style.maxWidth = urlActionMaxWidth + "px";
             this._actionText.style.maxWidth = urlActionMaxWidth + "px";
           }
         ]]></body>
       </method>
 
       <method name="handleOverUnderflow">
         <body>
--- a/toolkit/themes/linux/global/autocomplete.css
+++ b/toolkit/themes/linux/global/autocomplete.css
@@ -143,30 +143,32 @@ html|span.ac-tag {
   -moz-margin-end: 2px;
 }
 
 .ac-tags {
   -moz-margin-start: 0;
   -moz-margin-end: 4px;
 }
 
-html|span.ac-title-urlaction-separator {
+.ac-separator {
   padding-left: 0;
   padding-right: 6px;
 }
 
 /* Better align the URL/action with the title. */
 .ac-tags,
+.ac-separator,
 .ac-url,
 .ac-action {
   margin-bottom: -2px;
 }
 
 .ac-title-text,
 .ac-tags-text,
+.ac-separator-text,
 .ac-url-text,
 .ac-action-text,
 .ac-text-overflow-container {
   padding: 0 !important;
   margin: 0 !important;
 }
 
 /* ::::: textboxes inside toolbarpaletteitems ::::: */
--- a/toolkit/themes/osx/global/autocomplete.css
+++ b/toolkit/themes/osx/global/autocomplete.css
@@ -121,30 +121,32 @@ html|span.ac-tag {
   -moz-margin-end: 2px;
 }
 
 .ac-tags {
   -moz-margin-start: 0;
   -moz-margin-end: 4px;
 }
 
-html|span.ac-title-urlaction-separator {
+.ac-separator {
   -moz-margin-start: 0;
   -moz-margin-end: 6px;
 }
 
 /* Better align the URL/action with the title. */
 .ac-tags,
+.ac-separator,
 .ac-url,
 .ac-action {
   margin-bottom: -2px;
 }
 
 .ac-title-text,
 .ac-tags-text,
+.ac-separator-text,
 .ac-url-text,
 .ac-action-text,
 .ac-text-overflow-container {
   padding: 0 !important;
   margin: 0 !important;
 }
 
 /* ::::: textboxes inside toolbarpaletteitems ::::: */
--- a/toolkit/themes/windows/global/autocomplete.css
+++ b/toolkit/themes/windows/global/autocomplete.css
@@ -125,30 +125,32 @@ html|span.ac-tag {
   -moz-margin-end: 2px;
 }
 
 .ac-tags {
   -moz-margin-start: 0;
   -moz-margin-end: 4px;
 }
 
-html|span.ac-title-urlaction-separator {
+.ac-separator {
   padding-left: 0;
   padding-right: 6px;
 }
 
 /* Better align the URL/action with the title. */
 .ac-tags,
+.ac-separator,
 .ac-url,
 .ac-action {
   margin-bottom: -2px;
 }
 
 .ac-title-text,
 .ac-tags-text,
+.ac-separator-text,
 .ac-url-text,
 .ac-action-text,
 .ac-text-overflow-container {
   padding: 0 !important;
   margin: 0 !important;
 }
 
 /* ::::: textboxes inside toolbarpaletteitems ::::: */