Bug 1330732 - Show install warnings in about:debugging r?jdescottes, r?aswan
MozReview-Commit-ID: 8SFcYuln8w8
--- a/devtools/client/aboutdebugging/aboutdebugging.css
+++ b/devtools/client/aboutdebugging/aboutdebugging.css
@@ -325,16 +325,35 @@ button {
}
.addon-target-button:first-of-type {
/* Subtract the start padding so the button is still a bigger click target but
* lines up with the icon. */
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 {
direction: rtl;
max-width: 100%;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
--- a/devtools/client/aboutdebugging/components/addons/panel.js
+++ b/devtools/client/aboutdebugging/components/addons/panel.js
@@ -1,15 +1,16 @@
/* 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 { 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");
const AddonsControls = createFactory(require("./controls"));
const AddonTarget = createFactory(require("./target"));
const PanelHeader = createFactory(require("../panel-header"));
const TargetList = createFactory(require("../target-list"));
@@ -38,28 +39,33 @@ module.exports = createClass({
return {
extensions: [],
debugDisabled: false,
};
},
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);
Services.prefs.addObserver(REMOTE_ENABLED_PREF,
this.updateDebugStatus);
this.updateDebugStatus();
this.updateAddonsList();
},
componentWillUnmount() {
AddonManager.removeAddonListener(this);
+ Management.off("startup", this.updateAddonsList);
+
Services.prefs.removeObserver(CHROME_ENABLED_PREF,
this.updateDebugStatus);
Services.prefs.removeObserver(REMOTE_ENABLED_PREF,
this.updateDebugStatus);
},
updateDebugStatus() {
let debugDisabled =
@@ -76,16 +82,17 @@ module.exports = createClass({
return {
name: addon.name,
icon: addon.iconURL || ExtensionIcon,
addonID: addon.id,
addonActor: addon.actor,
temporarilyInstalled: addon.temporarilyInstalled,
url: addon.url,
manifestURL: addon.manifestURL,
+ warnings: addon.warnings,
};
});
this.setState({ extensions });
}, error => {
throw new Error("Client error while listing addons: " + error);
});
},
--- a/devtools/client/aboutdebugging/components/addons/target.js
+++ b/devtools/client/aboutdebugging/components/addons/target.js
@@ -65,46 +65,65 @@ function internalIDForTarget(target) {
{ href: target.manifestURL, target: "_blank", className: "manifest-url" },
Strings.GetStringFromName("manifestURL"),
),
)
),
];
}
-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({
displayName: "AddonTarget",
propTypes: {
client: PropTypes.instanceOf(DebuggerClient).isRequired,
debugDisabled: PropTypes.bool,
target: PropTypes.shape({
addonActor: PropTypes.string.isRequired,
addonID: PropTypes.string.isRequired,
icon: PropTypes.string,
name: PropTypes.string.isRequired,
temporarilyInstalled: PropTypes.bool,
url: PropTypes.string,
+ warnings: PropTypes.array,
}).isRequired
},
debug() {
let { target } = this.props;
debugAddon(target.addonID);
},
@@ -134,17 +153,17 @@ module.exports = createClass({
dom.div({ className: "target" },
dom.img({
className: "target-icon",
role: "presentation",
src: target.icon
}),
dom.span({ className: "target-name", title: target.name }, target.name)
),
- ...temporaryID(target),
+ showMessages(target),
dom.dl(
{ className: "addon-target-info" },
...filePathForTarget(target),
...internalIDForTarget(target),
),
dom.div({className: "addon-target-actions"},
dom.button({
className: "debug-button addon-target-button",
new file mode 100644
--- /dev/null
+++ b/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"
+ }
+}
--- a/devtools/client/aboutdebugging/test/browser.ini
+++ b/devtools/client/aboutdebugging/test/browser.ini
@@ -5,16 +5,17 @@ support-files =
head.js
addons/unpacked/bootstrap.js
addons/unpacked/install.rdf
addons/bad/manifest.json
addons/bug1273184.xpi
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
service-workers/empty-sw.js
service-workers/fetch-sw.html
service-workers/fetch-sw.js
service-workers/push-sw.html
service-workers/push-sw.js
--- a/devtools/client/aboutdebugging/test/browser_addons_debug_info.js
+++ b/devtools/client/aboutdebugging/test/browser_addons_debug_info.js
@@ -76,8 +76,37 @@ add_task(function* testTemporaryWebExten
let temporaryID = container.querySelector(".temporary-id-url");
ok(temporaryID, "Temporary ID message does appear");
yield uninstallAddon({document, id: addonId, name: addonName});
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);
+});
--- a/devtools/client/aboutdebugging/test/head.js
+++ b/devtools/client/aboutdebugging/test/head.js
@@ -1,20 +1,13 @@
/* Any copyright is dedicated to the Public Domain.
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";
// Load the shared-head file first.
Services.scriptloader.loadSubScript(
"chrome://mochitests/content/browser/devtools/client/framework/test/shared-head.js",
this);
@@ -267,16 +260,33 @@ function waitForInitialAddonList(documen
info("Actually, the add-ons have already loaded");
result = Promise.resolve();
} else {
result = waitForMutation(addonListContainer, { childList: true });
}
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.
* @param {Node} target
* @param {Object} mutationOptions
* @return {Promise}
*/
function waitForMutation(target, mutationOptions) {
--- a/devtools/server/actors/webextension-parent.js
+++ b/devtools/server/actors/webextension-parent.js
@@ -70,16 +70,17 @@ const WebExtensionParentActor = protocol
id: this.id,
name: this.addon.name,
url: this.addon.sourceURI ? this.addon.sourceURI.spec : undefined,
iconURL: this.addon.iconURL,
debuggable: this.addon.isDebuggable,
temporarilyInstalled: this.addon.temporarilyInstalled,
isWebExtension: true,
manifestURL: policy && policy.getURL("manifest.json"),
+ warnings: ExtensionParent.DebugUtils.getExtensionManifestWarnings(this.id),
};
},
connect() {
if (this._childFormPormise) {
return this._childFormPromise;
}
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -317,36 +317,58 @@ this.ExtensionData = class {
this.localeData = null;
this._promiseLocales = null;
this.apiNames = new Set();
this.dependencies = new Set();
this.permissions = new Set();
this.errors = [];
+ this.warnings = [];
}
get builtinMessages() {
return null;
}
get logger() {
let id = this.id || "<unknown>";
return Log.repository.getLogger(LOGGER_ID_BASE + id);
}
// Report an error about the extension's manifest file.
manifestError(message) {
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}`);
}
/**
* Returns the moz-extension: URL for the given path within this
* extension.
*
* Must not be called unless either the `id` or `uuid` property has
* already been set.
@@ -501,17 +523,17 @@ this.ExtensionData = class {
}
}).then(() => {
let context = {
url: this.baseURI && this.baseURI.spec,
principal: this.principal,
logError: error => {
- this.logger.warn(`Loading extension '${this.id}': Reading manifest: ${error}`);
+ this.manifestWarning(error);
},
preprocessors: {},
};
if (this.manifest.theme) {
let invalidProps = validateThemeManifest(Object.getOwnPropertyNames(this.manifest));
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -923,16 +923,24 @@ const DebugUtils = {
unwatchExtensionUpdated() {
if (this._extensionUpdatedWatcher) {
apiManager.off("ready", this._extensionUpdatedWatcher);
delete this._extensionUpdatedWatcher;
}
},
+ 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
* the devtools actor with the process where the extension is running.
*
* @param {WebExtensionParentActor} webExtensionParentActor
* The devtools actor that is retrieving the browser element.
*