Permalink
Browse files

Bug 839814: Returning undefined from a context listener should hide t…

…he menu item.
  • Loading branch information...
1 parent ef756d4 commit 11b766d803947adbc36e460bab5758c00c22be37 @Mossop committed Feb 10, 2013
Showing with 180 additions and 5 deletions.
  1. +5 −5 lib/sdk/context-menu.js
  2. +175 −0 test/test-context-menu.js
View
@@ -325,11 +325,11 @@ function hasMatchingContext(contexts, popupNode) {
}
// Gets the matched context from any worker for this item. If there is no worker
-// or no matched context then returns null.
+// or no matched context then returns false.
function getCurrentWorkerContext(item, popupNode) {
let worker = getItemWorkerForWindow(item, popupNode.ownerDocument.defaultView);
- if (!worker)
- return null;
+ if (!worker || !worker.anyContextListeners())
+ return true;
return worker.getMatchedContext(popupNode);
}
@@ -346,10 +346,10 @@ function isItemVisible(item, popupNode, defaultVisibility) {
return false;
let context = getCurrentWorkerContext(item, popupNode);
- if (typeof(context) === "string")
+ if (typeof(context) === "string" && context != "")
item.label = context;
- return context !== false;
+ return !!context;
}
// Gets the item's content script worker for a window, creating one if necessary
View
@@ -601,6 +601,143 @@ exports.testContentContextNoMatch = function (test) {
};
+// Content contexts that return undefined should cause their items to be absent
+// from the menu.
+exports.testContentContextUndefined = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let item = new loader.cm.Item({
+ label: "item",
+ contentScript: 'self.on("context", function () {});'
+ });
+
+ test.showMenu(null, function (popup) {
+ test.checkMenu([item], [item], []);
+ test.done();
+ });
+};
+
+
+// Content contexts that return an empty string should cause their items to be
+// absent from the menu and shouldn't wipe the label
+exports.testContentContextEmptyString = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let item = new loader.cm.Item({
+ label: "item",
+ contentScript: 'self.on("context", function () "");'
+ });
+
+ test.showMenu(null, function (popup) {
+ test.checkMenu([item], [item], []);
+ test.assertEqual(item.label, "item", "Label should still be correct");
+ test.done();
+ });
+};
+
+
+// If any content contexts returns true then their items should be present in
+// the menu.
+exports.testMultipleContentContextMatch1 = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let item = new loader.cm.Item({
+ label: "item",
+ contentScript: 'self.on("context", function () true); ' +
+ 'self.on("context", function () false);',
+ onMessage: function() {
+ test.fail("Should not have called the second context listener");
+ }
+ });
+
+ test.showMenu(null, function (popup) {
+ test.checkMenu([item], [], []);
+ test.done();
+ });
+};
+
+
+// If any content contexts returns true then their items should be present in
+// the menu.
+exports.testMultipleContentContextMatch2 = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let item = new loader.cm.Item({
+ label: "item",
+ contentScript: 'self.on("context", function () false); ' +
+ 'self.on("context", function () true);'
+ });
+
+ test.showMenu(null, function (popup) {
+ test.checkMenu([item], [], []);
+ test.done();
+ });
+};
+
+
+// If any content contexts returns a string then their items should be present
+// in the menu.
+exports.testMultipleContentContextString1 = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let item = new loader.cm.Item({
+ label: "item",
+ contentScript: 'self.on("context", function () "new label"); ' +
+ 'self.on("context", function () false);'
+ });
+
+ test.showMenu(null, function (popup) {
+ test.checkMenu([item], [], []);
+ test.assertEqual(item.label, "new label", "Label should have changed");
+ test.done();
+ });
+};
+
+
+// If any content contexts returns a string then their items should be present
+// in the menu.
+exports.testMultipleContentContextString2 = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let item = new loader.cm.Item({
+ label: "item",
+ contentScript: 'self.on("context", function () false); ' +
+ 'self.on("context", function () "new label");'
+ });
+
+ test.showMenu(null, function (popup) {
+ test.checkMenu([item], [], []);
+ test.assertEqual(item.label, "new label", "Label should have changed");
+ test.done();
+ });
+};
+
+
+// If many content contexts returns a string then the first should take effect
+exports.testMultipleContentContextString3 = function (test) {
+ test = new TestHelper(test);
+ let loader = test.newLoader();
+
+ let item = new loader.cm.Item({
+ label: "item",
+ contentScript: 'self.on("context", function () "new label 1"); ' +
+ 'self.on("context", function () "new label 2");'
+ });
+
+ test.showMenu(null, function (popup) {
+ test.checkMenu([item], [], []);
+ test.assertEqual(item.label, "new label 1", "Label should have changed");
+ test.done();
+ });
+};
+
+
// 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) {
@@ -677,6 +814,44 @@ exports.testContentContextNoMatchActiveElement = function (test) {
};
+// Content contexts that return undefined 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 () {});'
+ }),
+ new loader.cm.Item({
+ label: "item 2",
+ context: undefined,
+ contentScript: 'self.on("context", function () {});'
+ }),
+ // 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 () {});'
+ }),
+ new loader.cm.Item({
+ label: "item 4",
+ context: [loader.cm.PageContext()],
+ contentScript: 'self.on("context", function () {});'
+ })
+ ];
+
+ 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) {

0 comments on commit 11b766d

Please sign in to comment.