Bug 1486166 - Set the tooltip text on the parent of the urlbar input. r=dao
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 25 Aug 2018 07:11:27 +0200
changeset 488739 20a43066e367897769f5c155b1daba955cfaeea7
parent 488738 d5203d9dc33b2f829f621c30e99dbfa5a4bed32c
child 488740 15e5faa7e1d9a290118f9c05c1480d1f6f3b9a6b
push id9734
push usershindli@mozilla.com
push dateThu, 30 Aug 2018 12:18:07 +0000
treeherdermozilla-beta@71c71ab3afae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1486166, 1440682
milestone63.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 1486166 - Set the tooltip text on the parent of the urlbar input. r=dao The reason bug 1440682 broke this was because I moved all the code to nsXULElement. However, there was a way for non-XUL elements to get XUL tooltips before that, which was via the RestyleManager mechanism to handle attribute mutations. So the behavior before that patch was that non-XUL elements that got the attribute dynamically added or removed before that patch got their tooltips, like the HTML input in the toolbar, but if you specified the attributes statically in the markup, or while the element was somehow outside of the document or what not, it would never work. Given that, this looks completely unintentional, and the fact that this ever worked was a bit lucky. Chances are we eventually want tooltip support for HTML elements in chrome documents, but it is pretty likely that we want to use the HTML tooltip infrastructure instead of nsXULTooltipListener, which is kind of an odd thing. Thus, for now patch the code so that it sets it on the container of the <input>, which is a XUL box that takes the same space as the <input>, instead of moving all the XUL tooltip support to work on non-XUL elements. Also, while at it, remove references to inputtooltiptext, since I didn't find a single reference in the code that would set this attribute ever. Differential Revision: https://phabricator.services.mozilla.com/D4294
browser/base/content/urlbarBindings.xml
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -23,27 +23,27 @@ file, You can obtain one at http://mozil
 
   <binding id="urlbar" extends="chrome://global/content/bindings/autocomplete.xml#autocomplete">
 
     <content sizetopopup="pref">
       <xul:hbox flex="1" class="urlbar-textbox-container">
         <children includes="image|deck|stack|box"/>
         <xul:moz-input-box anonid="moz-input-box"
                   class="urlbar-input-box"
-                  flex="1" xbl:inherits="tooltiptext=inputtooltiptext">
+                  flex="1">
           <children/>
           <html:input anonid="scheme"
                       class="urlbar-scheme textbox-input"
                       required="required"
                       xbl:inherits="textoverflow,focused"/>
           <html:input anonid="input"
                       class="autocomplete-textbox urlbar-input textbox-input"
                       allowevents="true"
                       inputmode="mozAwesomebar"
-                      xbl:inherits="tooltiptext=inputtooltiptext,value,maxlength,disabled,size,readonly,placeholder,tabindex,accesskey,focused,textoverflow"/>
+                      xbl:inherits="value,maxlength,disabled,size,readonly,placeholder,tabindex,accesskey,focused,textoverflow"/>
         </xul:moz-input-box>
         <xul:image anonid="urlbar-go-button"
                    class="urlbar-go-button urlbar-icon"
                    onclick="gURLBar.handleCommand(event);"
                    tooltiptext="&goEndCap.tooltip;"
                    xbl:inherits="pageproxystate,parentfocused=focused,usertyping"/>
         <xul:dropmarker anonid="historydropmarker"
                         class="urlbar-history-dropmarker urlbar-icon chromeclass-toolbar-additional"
@@ -1053,23 +1053,27 @@ file, You can obtain one at http://mozil
           this.popup.overrideValue = "http://www." + url;
         ]]></body>
       </method>
 
       <method name="_initURLTooltip">
         <body><![CDATA[
           if (this.focused || !this.hasAttribute("textoverflow"))
             return;
-          this.inputField.setAttribute("tooltiptext", this.value);
+          // We set the tooltip text on the parent node instead of the input
+          // field because XUL tooltips only work on XUL elements.
+          //
+          // FIXME(bug 1486716): title should work on chrome documents instead.
+          this.inputField.parentNode.setAttribute("tooltiptext", this.value);
         ]]></body>
       </method>
 
       <method name="_hideURLTooltip">
         <body><![CDATA[
-          this.inputField.removeAttribute("tooltiptext");
+          this.inputField.parentNode.removeAttribute("tooltiptext");
         ]]></body>
       </method>
 
       <!-- Returns:
            null if there's a security issue and we should do nothing.
            a URL object if there is one that we're OK with loading,
            a text value otherwise.
            -->