Bug 1270462 - part2: tooltip toggle callback can resolve(false);r=bgrins,jsnajdr draft
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 05 May 2016 20:25:08 +0200
changeset 364351 279bab30f631a3a65a93b52226c6980210abf2f1
parent 364350 5dcca2d5887ffc98fec621092640073a0909c13f
child 520246 c186fed77b6af5899d5bba04c43f3e14390f2cf8
push id17416
push userjdescottes@mozilla.com
push dateFri, 06 May 2016 12:57:02 +0000
reviewersbgrins, jsnajdr
bugs1270462
milestone49.0a1
Bug 1270462 - part2: tooltip toggle callback can resolve(false);r=bgrins,jsnajdr Previously, the targetNodeCb used in TooltipToggle had an inconsistent API. If returning synchronously, "false" would prevent the tooltip from appearing. However, if using a promise, resolving "false" would still show the tooltip. It was needed to reject the promise in this case to prevent the tooltip from being displayed. This commit makes TooltipToggle always expect a consistent return value from this callback, whether it is synchronous, or using promises. - true -> show the tooltip on the event target - DOM node -> show the tooltip on the provided node - false (or falsy value) -> do not show the tooltip MozReview-Commit-ID: 7PIPwBJxjWO
devtools/client/inspector/markup/markup.js
devtools/client/inspector/shared/style-inspector-overlays.js
devtools/client/netmonitor/netmonitor-view.js
devtools/client/shared/widgets/tooltip/TooltipToggle.js
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -2374,39 +2374,42 @@ MarkupElementContainer.prototype = Herit
    * the mouse hovers over a target in the markup-view.
    * Checks if the target is indeed something we want to have an image tooltip
    * preview over and, if so, inserts content into the tooltip.
    *
    * @return {Promise} that resolves when the content has been inserted or
    *         rejects if no preview is required. This promise is then used by
    *         Tooltip.js to decide if/when to show the tooltip
    */
-  isImagePreviewTarget: function (target, tooltip) {
+  isImagePreviewTarget: Task.async(function* (target, tooltip) {
     // Is this Element previewable.
     if (!this.isPreviewable()) {
-      return promise.reject(false);
+      return false;
     }
 
     // If the Element has an src attribute, the tooltip is shown when hovering
     // over the src url. If not, the tooltip is shown when hovering over the tag
     // name.
     let src = this.editor.getAttributeElement("src");
     let expectedTarget = src ? src.querySelector(".link") : this.editor.tag;
     if (target !== expectedTarget) {
-      return promise.reject(false);
+      return false;
     }
 
-    return this._getPreview().then(({data, size}) => {
+    try {
+      let {data, size} = yield this._getPreview();
       // The preview is ready.
       tooltip.setImageContent(data, size);
-    }, () => {
+    } catch (e) {
       // Indicate the failure but show the tooltip anyway.
       tooltip.setBrokenImageContent();
-    });
-  },
+    }
+
+    return true;
+  }),
 
   copyImageDataUri: function () {
     // We need to send again a request to gettooltipData even if one was sent
     // for the tooltip, because we want the full-size image
     this.node.getImageData().then(data => {
       data.data.string().then(str => {
         clipboardHelper.copyString(str);
       });
--- a/devtools/client/inspector/shared/style-inspector-overlays.js
+++ b/devtools/client/inspector/shared/style-inspector-overlays.js
@@ -354,28 +354,29 @@ TooltipsOverlay.prototype = {
 
   /**
    * Executed by the tooltip when the pointer hovers over an element of the
    * view. Used to decide whether the tooltip should be shown or not and to
    * actually put content in it.
    * Checks if the hovered target is a css value we support tooltips for.
    *
    * @param {DOMNode} target The currently hovered node
+   * @return {Promise}
    */
-  _onPreviewTooltipTargetHover: function (target) {
+  _onPreviewTooltipTargetHover: Task.async(function* (target) {
     let nodeInfo = this.view.getNodeInfo(target);
     if (!nodeInfo) {
       // The hovered node isn't something we care about
-      return promise.reject(false);
+      return false;
     }
 
     let type = this._getTooltipType(nodeInfo);
     if (!type) {
       // There is no tooltip type defined for the hovered node
-      return promise.reject(false);
+      return false;
     }
 
     if (this.isRuleView && this.colorPicker.tooltip.isShown()) {
       this.colorPicker.revert();
       this.colorPicker.hide();
     }
 
     if (this.isRuleView && this.cubicBezier.tooltip.isShown()) {
@@ -393,27 +394,29 @@ TooltipsOverlay.prototype = {
     }
 
     let inspector = this.view.inspector;
 
     if (type === TOOLTIP_IMAGE_TYPE) {
       let dim = Services.prefs.getIntPref(PREF_IMAGE_TOOLTIP_SIZE);
       // nodeInfo contains an absolute uri
       let uri = nodeInfo.value.url;
-      return this.previewTooltip.setRelativeImageContent(uri,
+      yield this.previewTooltip.setRelativeImageContent(uri,
         inspector.inspector, dim);
+      return true;
     }
 
     if (type === TOOLTIP_FONTFAMILY_TYPE) {
-      return this.previewTooltip.setFontFamilyContent(nodeInfo.value.value,
+      yield this.previewTooltip.setFontFamilyContent(nodeInfo.value.value,
         inspector.selection.nodeFront);
+      return true;
     }
 
-    return undefined;
-  },
+    return false;
+  }),
 
   _onNewSelection: function () {
     if (this.previewTooltip) {
       this.previewTooltip.hide();
     }
 
     if (this.colorPicker) {
       this.colorPicker.hide();
--- a/devtools/client/netmonitor/netmonitor-view.js
+++ b/devtools/client/netmonitor/netmonitor-view.js
@@ -2216,42 +2216,41 @@ RequestsMenuView.prototype = Heritage.ex
   /**
    * The predicate used when deciding whether a popup should be shown
    * over a request item or not.
    *
    * @param nsIDOMNode target
    *        The element node currently being hovered.
    * @param object tooltip
    *        The current tooltip instance.
+   * @return {Promise}
    */
-  _onHover: function (target, tooltip) {
+  _onHover: Task.async(function* (target, tooltip) {
     let requestItem = this.getItemForElement(target);
     if (!requestItem || !requestItem.attachment.responseContent) {
-      return null;
+      return false;
     }
 
     let hovered = requestItem.attachment;
     let { mimeType, text, encoding } = hovered.responseContent.content;
 
     if (mimeType && mimeType.includes("image/") && (
       target.classList.contains("requests-menu-icon") ||
       target.classList.contains("requests-menu-file"))) {
-      return gNetwork.getString(text).then(string => {
-        let anchor = $(".requests-menu-icon", requestItem.target);
-        let src = formDataURI(mimeType, encoding, string);
-
-        tooltip.setImageContent(src, {
-          maxDim: REQUESTS_TOOLTIP_IMAGE_MAX_DIM
-        });
-
-        return anchor;
+      let string = yield gNetwork.getString(text);
+      let anchor = $(".requests-menu-icon", requestItem.target);
+      let src = formDataURI(mimeType, encoding, string);
+
+      tooltip.setImageContent(src, {
+        maxDim: REQUESTS_TOOLTIP_IMAGE_MAX_DIM
       });
+      return anchor;
     }
-    return undefined;
-  },
+    return false;
+  }),
 
   /**
    * A handler that opens the security tab in the details view if secure or
    * broken security indicator is clicked.
    */
   _onSecurityIconClick: function (e) {
     let state = this.selectedItem.attachment.securityState;
     if (state !== "insecure") {
--- a/devtools/client/shared/widgets/tooltip/TooltipToggle.js
+++ b/devtools/client/shared/widgets/tooltip/TooltipToggle.js
@@ -1,16 +1,18 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
+const {Task} = require("resource://gre/modules/Task.jsm");
+
 const DEFAULT_SHOW_DELAY = 50;
 
 /**
  * Tooltip helper designed to show/hide the tooltip when the mouse hovers over
  * particular nodes.
  *
  * This works by tracking mouse movements on a base container node (baseNode)
  * and showing the tooltip when the mouse stops moving. A callback can be
@@ -41,24 +43,24 @@ TooltipToggle.prototype = {
    *   a tooltip.
    *
    * Note that if you call this function a second time, it will itself call
    * stop() before adding mouse tracking listeners again.
    *
    * @param {node} baseNode
    *        The container for all target nodes
    * @param {Function} targetNodeCb
-   *        A function that accepts a node argument and returns true or false
-   *        (or a promise that resolves or rejects) to signify if the tooltip
-   *        should be shown on that node or not.
-   *        If the promise rejects, it must reject `false` as value.
-   *        Any other value is going to be logged as unexpected error.
-   *        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.
+   *        A function that accepts a node argument and that checks if a tooltip
+   *        should be displayed. Possible return values are:
+   *        - false (or a falsy value) if the tooltip should not be displayed
+   *        - true if the tooltip should be displayed
+   *        - a DOM node to display the tooltip on the returned anchor
+   *        The function can also return a promise that will resolve to one of
+   *        the values listed above.
+   *        If omitted, the tooltip will be shown everytime.
    * @param {Number} showDelay
    *        An optional delay that will be observed before showing the tooltip.
    *        Defaults to DEFAULT_SHOW_DELAY.
    */
   start: function (baseNode, targetNodeCb, showDelay = DEFAULT_SHOW_DELAY) {
     this.stop();
 
     if (!baseNode) {
@@ -97,53 +99,42 @@ TooltipToggle.prototype = {
   _onMouseMove: function (event) {
     if (event.target !== this._lastHovered) {
       this.tooltip.hide();
       this._lastHovered = event.target;
 
       this.win.clearTimeout(this.toggleTimer);
       this.toggleTimer = this.win.setTimeout(() => {
         this.isValidHoverTarget(event.target).then(target => {
+          if (target === null) {
+            return;
+          }
           this.tooltip.show(target);
         }, reason => {
-          if (reason === false) {
-            // isValidHoverTarget rejects with false if the tooltip should
-            // not be shown. This can be safely ignored.
-            return;
-          }
           console.error("isValidHoverTarget rejected with unexpected reason:");
           console.error(reason);
         });
       }, this._showDelay);
     }
   },
 
   /**
    * Is the given target DOMNode a valid node for toggling the tooltip on hover.
    * This delegates to the user-defined _targetNodeCb callback.
-   * @return a promise that resolves or rejects depending if the tooltip should
-   * be shown or not. If it resolves, it does to the actual anchor to be used
+   * @return {Promise} a promise that will resolve the anchor to use for the
+   *         tooltip or null if no valid target was found.
    */
-  isValidHoverTarget: function (target) {
-    // Execute the user-defined callback which should return either true/false
-    // or a promise that resolves or rejects
-    let res = this._targetNodeCb(target, this.tooltip);
+  isValidHoverTarget: Task.async(function* (target) {
+    let res = yield this._targetNodeCb(target, this.tooltip);
+    if (res) {
+      return res.nodeName ? res : target;
+    }
 
-    // The callback can additionally return a DOMNode to replace the anchor of
-    // the tooltip when shown
-    if (res && res.then) {
-      return res.then(arg => {
-        return arg && arg.nodeName ? arg : target;
-      });
-    }
-    let newTarget = res && res.nodeName ? res : target;
-    return new Promise((resolve, reject) => {
-      res ? resolve(newTarget) : reject(false);
-    });
-  },
+    return null;
+  }),
 
   _onMouseLeave: function () {
     this.win.clearTimeout(this.toggleTimer);
     this._lastHovered = null;
     this.tooltip.hide();
   },
 
   destroy: function () {