Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Only apply the implicit PageContext if there are no declarative conte…

…xts and no context listeners in the worker.
  • Loading branch information...
commit 974df0403a13a706a6d762836ac1acdfa65ac9bc 1 parent 7882a36
@Mossop authored
Showing with 92 additions and 9 deletions.
  1. +15 −8 lib/sdk/context-menu.js
  2. +77 −1 test/test-context-menu.js
View
23 lib/sdk/context-menu.js
@@ -291,6 +291,11 @@ let menuRules = mix(labelledItemRules, {
});
let ContextWorker = Worker.compose({
+ //Returns true if any context listeners are defined in the worker's port.
+ anyContextListeners: function anyContextListeners() {
+ return this._contentWorker.hasListenerFor("context");
+ },
+
// Calls the context workers context listeners and returns the first result
// that is either a string or a value that evaluates to true. If all of the
// listeners returned false then returns false. If there are no listeners
@@ -311,16 +316,12 @@ let ContextWorker = Worker.compose({
// Returns true if any contexts match. If there are no contexts then a
// PageContext is tested instead
function hasMatchingContext(contexts, popupNode) {
- if (contexts.length) {
- for (let context in contexts) {
- if (!context.isCurrent(popupNode))
- return false;
- }
-
- return true;
+ for (let context in contexts) {
+ if (!context.isCurrent(popupNode))
+ return false;
}
- return PageContext().isCurrent(popupNode);
+ return true;
}
// Gets the matched context from any worker for this item. If there is no worker
@@ -335,6 +336,12 @@ function getCurrentWorkerContext(item, popupNode) {
// Tests whether an item should be visible or not based on its contexts and
// content scripts
function isItemVisible(item, popupNode) {
+ if (!item.context.length) {
+ let worker = getItemWorkerForWindow(item, popupNode.ownerDocument.defaultView);
+ if (!worker || !worker.anyContextListeners())
+ return PageContext().isCurrent(popupNode);
+ }
+
if (!hasMatchingContext(item.context, popupNode))
return false;
View
78 test/test-context-menu.js
@@ -421,7 +421,7 @@ exports.testURLContextRemove = function (test) {
let item = loader.cm.Item({
label: "item",
context: context,
- contentScript: 'self.postMessage("ok");',
+ contentScript: 'self.postMessage("ok"); self.on("context", function () true);',
onMessage: function (msg) {
test.assert(shouldBeEvaled,
"content script should be evaluated when expected");
@@ -533,6 +533,82 @@ exports.testContentContextNoMatch = function (test) {
};
+// Content contexts that return true should cause their items to be present
+// in the menu when context clicking an active element.
+exports.testContentContextMatchActiveElement = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let items = [
+ new loader.cm.Item({
+ label: "item 1",
+ contentScript: 'self.on("context", function () true);'
+ }),
+ new loader.cm.Item({
+ label: "item 2",
+ context: undefined,
+ contentScript: 'self.on("context", function () true);'
+ }),
+ // These items will always be hidden by the declarative usage of PageContext
+ new loader.cm.Item({
+ label: "item 3",
+ context: loader.cm.PageContext(),
+ contentScript: 'self.on("context", function () true);'
+ }),
+ new loader.cm.Item({
+ label: "item 4",
+ context: [loader.cm.PageContext()],
+ contentScript: 'self.on("context", function () true);'
+ })
+ ];
+
+ test.withTestDoc(function (window, doc) {
+ test.showMenu(doc.getElementById("image"), function (popup) {
+ test.checkMenu(items, [items[2], items[3]], []);
+ test.done();
+ });
+ });
+};
+
+
+// Content contexts that return false should cause their items to be absent
+// from the menu when context clicking an active element.
+exports.testContentContextNoMatchActiveElement = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let items = [
+ new loader.cm.Item({
+ label: "item 1",
+ contentScript: 'self.on("context", function () false);'
+ }),
+ new loader.cm.Item({
+ label: "item 2",
+ context: undefined,
+ contentScript: 'self.on("context", function () false);'
+ }),
+ // These items will always be hidden by the declarative usage of PageContext
+ new loader.cm.Item({
+ label: "item 3",
+ context: loader.cm.PageContext(),
+ contentScript: 'self.on("context", function () false);'
+ }),
+ new loader.cm.Item({
+ label: "item 4",
+ context: [loader.cm.PageContext()],
+ contentScript: 'self.on("context", function () false);'
+ })
+ ];
+
+ test.withTestDoc(function (window, doc) {
+ test.showMenu(doc.getElementById("image"), function (popup) {
+ test.checkMenu(items, items, []);
+ test.done();
+ });
+ });
+};
+
+
// Content contexts that return a string should cause their items to be present
// in the menu and the items' labels to be updated.
exports.testContentContextMatchString = function (test) {
Please sign in to comment.
Something went wrong with that request. Please try again.