Bug 1449200: Return icon URL for all sizes when processing an icon string. r=Gijs
authorKris Maglione <maglione.k@gmail.com>
Thu, 29 Mar 2018 17:01:06 -0700
changeset 410850 e9052a77c9906a9be02b8442c10346c20fcd486d
parent 410849 78d53646681efbf4f262555b2cf93a5efae26374
child 410851 b537ebde3ae1393efb0d8ff29ff6838f4b02b914
push id33743
push userncsoregi@mozilla.com
push dateSat, 31 Mar 2018 10:06:30 +0000
treeherdermozilla-central@8063b0c54b88 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1449200
milestone61.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 1449200: Return icon URL for all sizes when processing an icon string. r=Gijs The CSS for page action icons doesn't handle fallback when only one variable is defined, so for widgets that don't define their icons using CSS, we always need to provide both. MozReview-Commit-ID: 7UgMSVS3W6K
browser/modules/PageActions.jsm
--- a/browser/modules/PageActions.jsm
+++ b/browser/modules/PageActions.jsm
@@ -687,19 +687,20 @@ Action.prototype = {
           "--pageAction-image-16px": escapeCSSURL(this._iconURLForSize(urls, 16)),
           "--pageAction-image-32px": escapeCSSURL(this._iconURLForSize(urls, 32)),
         });
         this._iconProperties.set(urls, props);
       }
       return props;
     }
 
+    let cssURL = urls ? escapeCSSURL(urls) : null;
     return Object.freeze({
-      "--pageAction-image-16px": null,
-      "--pageAction-image-32px": urls ? escapeCSSURL(urls) : null,
+      "--pageAction-image-16px": cssURL,
+      "--pageAction-image-32px": cssURL,
     });
   },
 
   /**
    * The action's title (string)
    */
   getTitle(browserWindow = null) {
     return this._getProperties(browserWindow).title;
@@ -802,41 +803,30 @@ Action.prototype = {
     return this._wantsIframe || false;
   },
 
   get labelForHistogram() {
     return this._labelForHistogram || this._id;
   },
 
   /**
-   * Returns the URL of the best icon to use given a preferred size.  The best
-   * icon is the one with the smallest size that's equal to or bigger than the
-   * preferred size.  Returns null if the action has no icon URL.
+   * Selects the best matching icon from the given URLs object for the
+   * given preferred size.
    *
-   * @param  peferredSize (number, required)
-   *         The icon size you prefer.
-   * @return The URL of the best icon, or null.
-   */
-  iconURLForSize(preferredSize, browserWindow) {
-    let iconURL = this.getIconURL(browserWindow);
-    if (!iconURL) {
-      return null;
-    }
-    if (typeof(iconURL) == "string") {
-      return iconURL;
-    }
-    if (typeof(iconURL) == "object") {
-      return this._iconURLForSize(iconURL, preferredSize);
-    }
-    return null;
-  },
-
-  /**
-   * Selects the best matching icon from the given URLs object for the
-   * given preferred size, as described in {@see iconURLForSize}.
+   * @param {object} urls
+   *        An object containing square icons of various sizes. The name
+   *        of each property is its width, and the value is its image URL.
+   * @param {integer} peferredSize
+   *        The preferred icon width. The most appropriate icon in the
+   *        urls object will be chosen to match that size. An exact
+   *        match will be preferred, followed by an icon exactly double
+   *        the size, followed by the smallest icon larger than the
+   *        preferred size, followed by the largest available icon.
+   * @returns {string}
+   *        The chosen icon URL.
    */
   _iconURLForSize(urls, preferredSize) {
     // This case is copied from ExtensionParent.jsm so that our image logic is
     // the same, so that WebExtensions page action tests that deal with icons
     // pass.
     let bestSize = null;
     if (urls[preferredSize]) {
       bestSize = preferredSize;