Bug 931845 - Shows image tooltips immediately and with no animation. r=bgrins
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 18 Nov 2013 13:03:20 -0500
changeset 156019 9cbb559661cd8072084b3b69e4e7ecf567c503e4
parent 156018 6c8dc8b0add46b6cc6f84580c5db932d4d9f69b2
child 156020 49b74d3271dfbdb827b048c0c6c54beac8ffae34
push id3505
push userryanvm@gmail.com
push dateMon, 18 Nov 2013 18:05:04 +0000
treeherderfx-team@14f9233f4b79 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs931845
milestone28.0a1
Bug 931845 - Shows image tooltips immediately and with no animation. r=bgrins
browser/devtools/shared/widgets/Tooltip.js
browser/devtools/styleinspector/rule-view.js
browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js
browser/themes/shared/devtools/common.inc.css
--- a/browser/devtools/shared/widgets/Tooltip.js
+++ b/browser/devtools/shared/widgets/Tooltip.js
@@ -19,16 +19,17 @@ Cu.import("resource:///modules/devtools/
 const GRADIENT_RE = /\b(repeating-)?(linear|radial)-gradient\(((rgb|hsl)a?\(.+?\)|[^\)])+\)/gi;
 const BORDERCOLOR_RE = /^border-[-a-z]*color$/ig;
 const BORDER_RE = /^border(-(top|bottom|left|right))?$/ig;
 const BACKGROUND_IMAGE_RE = /url\([\'\"]?(.*?)[\'\"]?\)/;
 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.
  *
  * This widget is intended at any tool that may need to show rich content in the
  * form of floating panels.
  * A common use case is image previewing in the CSS rule view, but more complex
  * use cases may include color pickers, object inspection, etc...
@@ -186,18 +187,22 @@ Tooltip.prototype = {
 
   /**
    * Show the tooltip. It might be wise to append some content first if you
    * don't want the tooltip to be empty. You may access the content of the
    * tooltip by setting a XUL node to t.content.
    * @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,
     x = this.defaultOffsetX,
     y = this.defaultOffsetY) {
     this.panel.hidden = false;
     this.panel.openPopup(anchor, position, x, y);
   },
@@ -234,24 +239,24 @@ Tooltip.prototype = {
         this["_onPopup" + event], false);
     }
 
     let win = this.doc.querySelector("window");
     win.removeEventListener("keypress", this._onKeyPress, false);
 
     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();
-    }
   },
 
   /**
    * Show/hide the tooltip when the mouse hovers over particular nodes.
    *
    * 2 Ways to make this work:
    * - Provide a single node to attach the tooltip to, as the baseNode, and
    *   omit the second targetNodeCb argument
@@ -274,27 +279,23 @@ Tooltip.prototype = {
    * @param {Function} targetNodeCb
    *        A function that accepts a node argument and returns true or false
    *        to signify if the tooltip should be shown on that node or not.
    *        Additionally, the function receives a second argument which is the
    *        tooltip instance itself, to be used to add/modify the content of the
    *        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);
 
     this._onBaseNodeMouseMove = this._onBaseNodeMouseMove.bind(this);
     this._onBaseNodeMouseLeave = this._onBaseNodeMouseLeave.bind(this);
 
     baseNode.addEventListener("mousemove", this._onBaseNodeMouseMove, false);
@@ -317,36 +318,33 @@ Tooltip.prototype = {
     this._basedNode = null;
     this._targetNodeCb = null;
     this._lastHovered = null;
   },
 
   _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);
       });
     }
   },
 
   _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();
   },
 
   /**
    * Set the content of this tooltip. Will first empty the tooltip and then
    * append the new content element.
    * Consider using one of the set<type>Content() functions instead.
    * @param {node} content
    *        A node that can be appended in the tooltip XUL element
@@ -360,17 +358,17 @@ Tooltip.prototype = {
 
   get content() {
     return this.panel.firstChild;
   },
 
   /**
    * Sets some text as the content of this tooltip.
    *
-   * @param string[] messages
+   * @param {string[]} messages
    *        A list of text messages.
    */
   setTextContent: function(...messages) {
     let vbox = this.doc.createElement("vbox");
     vbox.className = "devtools-tooltip-simple-text-container";
     vbox.setAttribute("flex", "1");
 
     for (let text of messages) {
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -1120,33 +1120,27 @@ CssRuleView.prototype = {
     popupset.appendChild(this._contextmenu);
   },
 
   /**
    * Verify that target is indeed a css value we want a tooltip on, and if yes
    * 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
     this.previewTooltip.setCssBackgroundImageContent(property.value, href);
 
     // Hide the color picker tooltip if shown and revert changes
     this.colorPicker.revert();
     this.colorPicker.hide();
--- a/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js
+++ b/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js
@@ -17,18 +17,17 @@ const PAGE_CONTENT = [
   '  }',
   '  .test-element {',
   '    font-family: verdana;',
   '    color: #333;',
   '    background: url(chrome://global/skin/icons/warning-64.png) no-repeat left center;',
   '    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() {
   waitForExplicitFinish();
 
   gBrowser.selectedTab = gBrowser.addTab();
   gBrowser.selectedBrowser.addEventListener("load", function(evt) {
     gBrowser.selectedBrowser.removeEventListener(evt.type, arguments.callee, true);
@@ -45,18 +44,16 @@ function createDocument() {
   openRuleView((aInspector, aRuleView) => {
     inspector = aInspector;
     ruleView = aRuleView;
     startTests();
   });
 }
 
 function startTests() {
-  // let testElement = contentDoc.querySelector(".test-element");
-
   inspector.selection.setNode(contentDoc.body);
   inspector.once("inspector-updated", testBodyRuleView);
 }
 
 function endTests() {
   contentDoc = inspector = ruleView = computedView = null;
   gBrowser.removeCurrentTab();
   finish();
@@ -76,58 +73,80 @@ function testBodyRuleView() {
 
   let panel = ruleView.previewTooltip.panel;
 
   // Check that the rule view has a tooltip and that a XUL panel has been created
   ok(ruleView.previewTooltip, "Tooltip instance exists");
   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");
 
     ruleView.previewTooltip.hide();
 
     inspector.selection.setNode(contentDoc.querySelector(".test-element"));
     inspector.once("inspector-updated", testDivRuleView);
   });
 }
 
 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
   assertTooltipShownOn(ruleView.previewTooltip, uriSpan, () => {
     let images = panel.getElementsByTagName("image");
     is(images.length, 1, "Tooltip contains an image");
     ok(images[0].src === "chrome://global/skin/icons/warning-64.png");
 
     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");
 
   inspector.sidebar.select("computedview");
   computedView = inspector.sidebar.getWindowForTab("computedview").computedview.view;
   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");
     is(images.length, 1, "Tooltip contains an image");
     ok(images[0].src === "chrome://global/skin/icons/warning-64.png");
 
     computedView.tooltip.hide();
 
--- a/browser/themes/shared/devtools/common.inc.css
+++ b/browser/themes/shared/devtools/common.inc.css
@@ -110,20 +110,29 @@
   .devtools-responsive-container > .devtools-sidebar-tabs {
     min-height: 35vh;
     max-height: 75vh;
   }
 }
 
 /* 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. */
   padding: 8px 12px;
   white-space: pre-wrap;
 }
 
 .devtools-tooltip-simple-text:first-child {