Bug 931845 - Shows image tooltips immediately and with no animation. r=bgrins

browser/devtools/shared/widgets/Tooltip.js

@@ -24,6 +24,7 @@ const XHTML_NS = "http://www.w3.org/1999/xhtml";
 const SPECTRUM_FRAME = "chrome://browser/content/devtools/spectrum-frame.xhtml";
 const ESCAPE_KEYCODE = Ci.nsIDOMKeyEvent.DOM_VK_ESCAPE;
 const ENTER_KEYCODE = Ci.nsIDOMKeyEvent.DOM_VK_RETURN;
+const SHOW_TIMEOUT = 50;
 
 /**
  * Tooltip widget.
@@ -191,8 +192,12 @@ Tooltip.prototype = {
    * @param {node} anchor
    *        Which node should the tooltip be shown on
    * @param {string} position
+   *        Optional tooltip position. Defaults to before_start
    *        https://developer.mozilla.org/en-US/docs/XUL/PopupGuide/Positioning
-   *        Defaults to before_start
+   * @param {number} x
+   *        Optional x offset. Defaults to 0
+   * @param {number} y
+   *        Optional y offset. Defaults to 0
    */
   show: function(anchor,
     position = this.defaultPosition,
@@ -239,14 +244,14 @@ Tooltip.prototype = {
 
     this.content = null;
 
+    if (this._basedNode) {
+      this.stopTogglingOnHover();
+    }
+
     this.doc = null;
 
     this.panel.parentNode.removeChild(this.panel);
     this.panel = null;
-
-    if (this._basedNode) {
-      this.stopTogglingOnHover();
-    }
   },
 
   /**
@@ -279,17 +284,13 @@ Tooltip.prototype = {
    *        tooltip if needed. If omitted, the tooltip will be shown everytime.
    * @param {Number} showDelay
    *        An optional delay that will be observed before showing the tooltip.
-   *        Defaults to 750ms
+   *        Defaults to SHOW_TIMEOUT
    */
-  startTogglingOnHover: function(baseNode, targetNodeCb, showDelay = 750) {
+  startTogglingOnHover: function(baseNode, targetNodeCb, showDelay=SHOW_TIMEOUT) {
     if (this._basedNode) {
       this.stopTogglingOnHover();
     }
 
-    // If no targetNodeCb callback is provided, then we need to hide the tooltip
-    // on mouseleave since baseNode is the target node itself
-    this._hideOnMouseLeave = !targetNodeCb;
-
     this._basedNode = baseNode;
     this._showDelay = showDelay;
     this._targetNodeCb = targetNodeCb || (() => true);
@@ -322,7 +323,7 @@ Tooltip.prototype = {
   _onBaseNodeMouseMove: function(event) {
     if (event.target !== this._lastHovered) {
       this.hide();
-      this._lastHovered = null;
+      this._lastHovered = event.target;
       setNamedTimeout(this.uid, this._showDelay, () => {
         this._showOnHover(event.target);
       });
@@ -332,16 +333,13 @@ Tooltip.prototype = {
   _showOnHover: function(target) {
     if (this._targetNodeCb(target, this)) {
       this.show(target);
-      this._lastHovered = target;
     }
   },
 
   _onBaseNodeMouseLeave: function() {
     clearNamedTimeout(this.uid);
     this._lastHovered = null;
-    if (this._hideOnMouseLeave) {
-      this.hide();
-    }
+    this.hide();
   },
 
   /**
@@ -365,7 +363,7 @@ Tooltip.prototype = {
   /**
    * Sets some text as the content of this tooltip.
    *
-   * @param string[] messages
+   * @param {string[]} messages
    *        A list of text messages.
    */
   setTextContent: function(...messages) {

browser/devtools/styleinspector/rule-view.js

@@ -1125,23 +1125,17 @@ CssRuleView.prototype = {
    * prepare some content for the tooltip
    */
   _buildTooltipContent: function(target) {
-    let isValueWithImage = target.classList.contains("ruleview-propertyvalue") &&
-      target.querySelector(".theme-link");
-
     let isImageHref = target.classList.contains("theme-link") &&
       target.parentNode.classList.contains("ruleview-propertyvalue");
-    if (isImageHref) {
-      target = target.parentNode;
-    }
 
     // If the inplace-editor is visible or if this is not a background image
     // don't show the tooltip
-    if (!isImageHref && !isValueWithImage) {
+    if (!isImageHref) {
       return false;
     }
 
     // Retrieve the TextProperty for the hovered element
-    let property = target.textProperty;
+    let property = target.parentNode.textProperty;
     let href = property.rule.domRule.href;
 
     // Fill some content

browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js

@@ -22,8 +22,7 @@ const PAGE_CONTENT = [
   '    padding-left: 70px;',
   '  }',
   '</style>',
-  '<div class="test-element">test element</div>',
-  '<div class="test-element-2">test element 2</div>'
+  '<div class="test-element">test element</div>'
 ].join("\n");
 
 function test() {
@@ -50,8 +49,6 @@ function createDocument() {
 }
 
 function startTests() {
-  // let testElement = contentDoc.querySelector(".test-element");
-
   inspector.selection.setNode(contentDoc.body);
   inspector.once("inspector-updated", testBodyRuleView);
 }
@@ -81,9 +78,10 @@ function testBodyRuleView() {
   ok(panel, "XUL panel exists");
 
   // Get the background-image property inside the rule view
-  let {nameSpan, valueSpan} = getRuleViewProperty("background-image");
+  let {valueSpan} = getRuleViewProperty("background-image");
+  let uriSpan = valueSpan.querySelector(".theme-link");
   // And verify that the tooltip gets shown on this property
-  assertTooltipShownOn(ruleView.previewTooltip, valueSpan, () => {
+  assertTooltipShownOn(ruleView.previewTooltip, uriSpan, () => {
     let images = panel.getElementsByTagName("image");
     is(images.length, 1, "Tooltip contains an image");
     ok(images[0].src.indexOf("iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAYAAACqaXHe") !== -1, "The image URL seems fine");
@@ -99,7 +97,7 @@ function testDivRuleView() {
   let panel = ruleView.previewTooltip.panel;
 
   // Get the background property inside the rule view
-  let {nameSpan, valueSpan} = getRuleViewProperty("background");
+  let {valueSpan} = getRuleViewProperty("background");
   let uriSpan = valueSpan.querySelector(".theme-link");
 
   // And verify that the tooltip gets shown on this property
@@ -110,10 +108,31 @@ function testDivRuleView() {
 
     ruleView.previewTooltip.hide();
 
-    testComputedView();
+    testTooltipAppearsEvenInEditMode();
   });
 }
 
+function testTooltipAppearsEvenInEditMode() {
+  let panel = ruleView.previewTooltip.panel;
+
+  // Switch one field to edit mode
+  let brace = ruleView.doc.querySelector(".ruleview-ruleclose");
+  waitForEditorFocus(brace.parentNode, editor => {
+    // Now try to show the tooltip
+    let {valueSpan} = getRuleViewProperty("background");
+    let uriSpan = valueSpan.querySelector(".theme-link");
+    assertTooltipShownOn(ruleView.previewTooltip, uriSpan, () => {
+      is(ruleView.doc.activeElement, editor.input,
+        "Tooltip was shown in edit mode, and inplace-editor still focused");
+
+      ruleView.previewTooltip.hide();
+
+      testComputedView();
+    });
+  });
+  brace.click();
+}
+
 function testComputedView() {
   info("Testing tooltips in the computed view");
 
@@ -122,7 +141,7 @@ function testComputedView() {
   let doc = computedView.styleDocument;
 
   let panel = computedView.tooltip.panel;
-  let {nameSpan, valueSpan} = getComputedViewProperty("background-image");
+  let {valueSpan} = getComputedViewProperty("background-image");
 
   assertTooltipShownOn(computedView.tooltip, valueSpan, () => {
     let images = panel.getElementsByTagName("image");

browser/themes/shared/devtools/common.inc.css

@@ -115,10 +115,19 @@
 
 /* Tooltip widget (see browser/devtools/shared/widgets/Tooltip.js) */
 
-.devtools-tooltip.devtools-tooltip-panel .panel-arrowcontent {
+.devtools-tooltip .panel-arrowcontent {
   padding: 4px;
 }
 
+.devtools-tooltip .panel-arrowcontainer {
+  /* Reseting the transition used when panels are shown */
+  transition: none;
+  /* Panels slide up/down/left/right when they appear using a transform.
+  Since we want to remove the transition, we don't need to transform anymore
+  plus it can interfeer by causing mouseleave events on the underlying nodes */
+  transform: none;
+}
+
 .devtools-tooltip-simple-text {
   max-width: 400px;
   margin: 0 -4px; /* Compensate for the .panel-arrowcontent padding. */