Bug 1330732 - Show install warnings in about:debugging r=aswan,jdescottes

MozReview-Commit-ID: 8SFcYuln8w8

--HG--
extra : rebase_source : 5e9b51508bf7b7366b393b485ff1cbbd1c7e146e

devtools/client/aboutdebugging/aboutdebugging.css

@@ -333,6 +333,25 @@ button {
   margin-inline-start: -4px;
 }
 
+.addon-target-messages {
+  border-bottom: 1px solid rgba(0, 0, 0, 0.2);
+  margin-bottom: 16px;
+  padding: 0 0 12px 0;
+}
+
+.addon-target-message {
+  list-style-type: none;
+  padding: 4px 0;
+}
+
+.addon-target-message:first-of-type {
+  padding-top: 0;
+}
+
+.addon-target-warning-message {
+  color: #a47f00;
+}
+
 /* We want the ellipsis on the left-hand-side, so make the parent RTL
  * with an ellipsis and the child can be LTR. */
 .file-path {

devtools/client/aboutdebugging/components/addons/panel.js

@@ -5,6 +5,7 @@
 "use strict";
 
 const { AddonManager } = require("resource://gre/modules/AddonManager.jsm");
+const { Management } = require("resource://gre/modules/Extension.jsm");
 const { createFactory, createClass, DOM: dom, PropTypes } =
   require("devtools/client/shared/vendor/react");
 const Services = require("Services");
@@ -43,6 +44,9 @@ module.exports = createClass({
 
   componentDidMount() {
     AddonManager.addAddonListener(this);
+    // Listen to startup since that's when errors and warnings
+    // get populated on the extension.
+    Management.on("startup", this.updateAddonsList);
 
     Services.prefs.addObserver(CHROME_ENABLED_PREF,
       this.updateDebugStatus);
@@ -55,6 +59,8 @@ module.exports = createClass({
 
   componentWillUnmount() {
     AddonManager.removeAddonListener(this);
+    Management.off("startup", this.updateAddonsList);
+
     Services.prefs.removeObserver(CHROME_ENABLED_PREF,
       this.updateDebugStatus);
     Services.prefs.removeObserver(REMOTE_ENABLED_PREF,
@@ -81,6 +87,7 @@ module.exports = createClass({
             temporarilyInstalled: addon.temporarilyInstalled,
             url: addon.url,
             manifestURL: addon.manifestURL,
+            warnings: addon.warnings,
           };
         });
 

devtools/client/aboutdebugging/components/addons/target.js

@@ -87,21 +87,39 @@ function internalIDForTarget(target) {
   ];
 }
 
-function temporaryID(target) {
-  if (!isTemporaryID(target.addonID)) {
-    return [];
+function showMessages(target) {
+  const messages = [
+    ...warningMessages(target.warnings),
+    ...infoMessages(target),
+  ];
+  if (messages.length > 0) {
+    return dom.ul(
+      { className: "addon-target-messages" },
+      ...messages);
   }
+  return null;
+}
 
-  return [
-    dom.div({ className: "addons-tip" },
-      dom.span({ className: "addons-web-ext-tip" },
-        Strings.GetStringFromName("temporaryID")
-      ),
+function infoMessages(target) {
+  const messages = [];
+  if (isTemporaryID(target.addonID)) {
+    messages.push(dom.li(
+      { className: "addon-target-info-message addon-target-message" },
+      Strings.GetStringFromName("temporaryID"),
+      " ",
       dom.a({ href: TEMP_ID_URL, className: "temporary-id-url", target: "_blank" },
         Strings.GetStringFromName("temporaryID.learnMore")
-      )
-    )
-  ];
+      )));
+  }
+  return messages;
+}
+
+function warningMessages(warnings = []) {
+  return warnings.map((warning) => {
+    return dom.li(
+      { className: "addon-target-warning-message addon-target-message" },
+      warning);
+  });
 }
 
 module.exports = createClass({
@@ -117,6 +135,7 @@ module.exports = createClass({
       name: PropTypes.string.isRequired,
       temporarilyInstalled: PropTypes.bool,
       url: PropTypes.string,
+      warnings: PropTypes.array,
     }).isRequired
   },
 
@@ -156,7 +175,7 @@ module.exports = createClass({
         }),
         dom.span({ className: "target-name", title: target.name }, target.name)
       ),
-      ...temporaryID(target),
+      showMessages(target),
       dom.dl(
         { className: "addon-target-info" },
         ...filePathForTarget(target),

devtools/client/aboutdebugging/test/addons/test-devtools-webextension-unknown-prop/manifest.json

@@ -0,0 +1,14 @@
+{
+  "manifest_version": 2,
+  "name": "test-devtools-webextension-unknown-prop",
+  "version": "1.0",
+  "applications": {
+    "gecko": {
+      "id": "test-devtools-webextension-unknown-prop@mozilla.org"
+    }
+  },
+  "browser_actions": {
+    "default_title": "WebExtension Popup Debugging",
+    "default_popup": "popup.html"
+  }
+}

devtools/client/aboutdebugging/test/browser.ini

@@ -10,6 +10,7 @@ support-files =
   addons/test-devtools-webextension/*
   addons/test-devtools-webextension-nobg/*
   addons/test-devtools-webextension-noid/*
+  addons/test-devtools-webextension-unknown-prop/*
   service-workers/delay-sw.html
   service-workers/delay-sw.js
   service-workers/empty-sw.html

devtools/client/aboutdebugging/test/browser_addons_debug_info.js

@@ -87,3 +87,32 @@ add_task(function* testTemporaryWebExtension() {
 
   yield closeAboutDebugging(tab);
 });
+
+add_task(function* testUnknownManifestProperty() {
+  let addonId = "test-devtools-webextension-unknown-prop@mozilla.org";
+  let addonName = "test-devtools-webextension-unknown-prop";
+  let { tab, document } = yield openAboutDebugging("addons");
+
+  yield waitForInitialAddonList(document);
+  yield installAddon({
+    document,
+    path: "addons/test-devtools-webextension-unknown-prop/manifest.json",
+    name: addonName,
+    isWebExtension: true
+  });
+
+  let container = document.querySelector(`[data-addon-id="${addonId}"]`);
+
+  yield waitForInstallMessages(container);
+
+  let messages = container.querySelectorAll(".addon-target-message");
+  ok(messages.length === 1, "there is one message");
+  ok(messages[0].textContent.match(/Error processing browser_actions/),
+     "the message is helpful");
+  ok(messages[0].classList.contains("addon-target-warning-message"),
+     "the message is a warning");
+
+  yield uninstallAddon({document, id: addonId, name: addonName});
+
+  yield closeAboutDebugging(tab);
+});

devtools/client/aboutdebugging/test/head.js

@@ -2,14 +2,7 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /* eslint-env browser */
-/* exported openAboutDebugging, changeAboutDebuggingHash, closeAboutDebugging,
-   installAddon, uninstallAddon, waitForMutation, waitForContentMutation, assertHasTarget,
-   getServiceWorkerList, getTabList, openPanel, waitForInitialAddonList,
-   waitForServiceWorkerRegistered, unregisterServiceWorker,
-   waitForDelayedStartupFinished, setupTestAboutDebuggingWebExtension,
-   waitForServiceWorkerActivation, enableServiceWorkerDebugging,
-   getServiceWorkerContainer, promiseAddonEvent, installAddonWithManager, getAddonByID,
-   tearDownAddon */
+/* eslint no-unused-vars: [2, {"vars": "local"}] */
 /* import-globals-from ../../framework/test/shared-head.js */
 
 "use strict";
@@ -272,6 +265,23 @@ function waitForInitialAddonList(document) {
   return result;
 }
 
+function waitForInstallMessages(target) {
+  return new Promise(resolve => {
+    let observer = new MutationObserver((mutations) => {
+      const messageAdded = mutations.some((mutation) => {
+        return [...mutation.addedNodes].some((node) => {
+          return node.classList.contains("addon-target-messages");
+        });
+      });
+      if (messageAdded) {
+        observer.disconnect();
+        resolve();
+      }
+    });
+    observer.observe(target, { childList: true });
+  });
+}
+
 /**
  * Returns a promise that will resolve after receiving a mutation matching the
  * provided mutation options on the provided target.

devtools/server/actors/webextension-parent.js

@@ -75,6 +75,7 @@ const WebExtensionParentActor = protocol.ActorClassWithSpec(webExtensionSpec, {
       temporarilyInstalled: this.addon.temporarilyInstalled,
       isWebExtension: true,
       manifestURL: policy && policy.getURL("manifest.json"),
+      warnings: ExtensionParent.DebugUtils.getExtensionManifestWarnings(this.id),
     };
   },
 

toolkit/components/extensions/Extension.jsm

@@ -329,6 +329,7 @@ this.ExtensionData = class {
     this.permissions = new Set();
 
     this.errors = [];
+    this.warnings = [];
   }
 
   get builtinMessages() {
@@ -345,10 +346,31 @@ this.ExtensionData = class {
     this.packagingError(`Reading manifest: ${message}`);
   }
 
+  manifestWarning(message) {
+    this.packagingWarning(`Reading manifest: ${message}`);
+  }
+
   // Report an error about the extension's general packaging.
   packagingError(message) {
     this.errors.push(message);
-    this.logger.error(`Loading extension '${this.id}': ${message}`);
+    this.logError(message);
+  }
+
+  packagingWarning(message) {
+    this.warnings.push(message);
+    this.logWarning(message);
+  }
+
+  logWarning(message) {
+    this._logMessage(message, "warn");
+  }
+
+  logError(message) {
+    this._logMessage(message, "error");
+  }
+
+  _logMessage(message, severity) {
+    this.logger[severity](`Loading extension '${this.id}': ${message}`);
   }
 
   /**
@@ -503,7 +525,7 @@ this.ExtensionData = class {
         principal: this.principal,
 
         logError: error => {
-          this.logger.warn(`Loading extension '${this.id}': Reading manifest: ${error}`);
+          this.manifestWarning(error);
         },
 
         preprocessors: {},

toolkit/components/extensions/ExtensionParent.jsm

@@ -1017,6 +1017,14 @@ const DebugUtils = {
     }
   },
 
+  getExtensionManifestWarnings(id) {
+    const addon = GlobalManager.extensionMap.get(id);
+    if (addon) {
+      return addon.warnings;
+    }
+    return [];
+  },
+
 
   /**
    * Retrieve a XUL browser element which has been configured to be able to connect