Bug 549340 - reorganize ContentAreaClick, give it consistent return values and comments.

r=gavin a=blocking

browser/base/content/browser.js

@@ -4988,134 +4988,150 @@ function asyncOpenWebPanel(event)
  *    - gatherTextUnder
  */
 
- // Called whenever the user clicks in the content area,
- // except when left-clicking on links (special case)
- // should always return true for click to go through
- function contentAreaClick(event, fieldNormalClicks)
+/**
+ * Extracts linkNode and href for the current click target.
+ *
+ * @param event
+ *        The click event.
+ * @return [href, linkNode].
+ *
+ * @note linkNode will be null if the click wasn't on an anchor
+ *       element (or XLink).
+ */
+function hrefAndLinkNodeForClickEvent(event)
 {
-   if (!event.isTrusted || event.getPreventDefault()) {
-     return true;
+  function isHTMLLink(aNode)
+  {
+    return aNode instanceof HTMLAnchorElement ||
+           aNode instanceof HTMLAreaElement ||
+           aNode instanceof HTMLLinkElement;
   }
 
-   var target = event.target;
-   var linkNode;
-
-   if (target instanceof HTMLAnchorElement ||
-       target instanceof HTMLAreaElement ||
-       target instanceof HTMLLinkElement) {
-     if (target.hasAttribute("href"))
-       linkNode = target;
-
-     // xxxmpc: this is kind of a hack to work around a Gecko bug (see bug 266932)
-     // we're going to walk up the DOM looking for a parent link node,
-     // this shouldn't be necessary, but we're matching the existing behaviour for left click
-     var parent = target.parentNode;
-     while (parent) {
-       if (parent instanceof HTMLAnchorElement ||
-           parent instanceof HTMLAreaElement ||
-           parent instanceof HTMLLinkElement) {
-           if (parent.hasAttribute("href"))
-             linkNode = parent;
-       }
-       parent = parent.parentNode;
+  let linkNode;
+  if (isHTMLLink(event.target)) {
+    // This is a hack to work around Gecko bug 266932.
+    // Walk up the DOM looking for a parent link node, to match the existing
+    // behaviour for left click.
+    // TODO: this is no more needed and should be removed in bug 325652.
+    let node = event.target;
+    while (node) {
+      if (isHTMLLink(node) && node.hasAttribute("href"))
+        linkNode = node;
+      node = node.parentNode;
     }
   }
   else {
-     linkNode = event.originalTarget;
-     while (linkNode && !(linkNode instanceof HTMLAnchorElement))
-       linkNode = linkNode.parentNode;
+    let node = event.originalTarget;
+    while (node && !(node instanceof HTMLAnchorElement)) {
+      node = node.parentNode;
+    }
     // <a> cannot be nested.  So if we find an anchor without an
-     // href, there is no useful <a> around the target
-     if (linkNode && !linkNode.hasAttribute("href"))
-       linkNode = null;
+    // href, there is no useful <a> around the target.
+    if (node && node.hasAttribute("href"))
+      linkNode = node;
   }
-   var wrapper = null;
-   if (linkNode) {
-     wrapper = linkNode;
-     if (event.button == 0 && !event.ctrlKey && !event.shiftKey &&
-         !event.altKey && !event.metaKey) {
-       // A Web panel's links should target the main content area.  Do this
-       // if no modifier keys are down and if there's no target or the target equals
-       // _main (the IE convention) or _content (the Mozilla convention).
-       // XXX Now that markLinkVisited is gone, we may not need to field _main and
-       // _content here.
-       target = wrapper.getAttribute("target");
-       if (fieldNormalClicks &&
-           (!target || target == "_content" || target  == "_main"))
-         // IE uses _main, SeaMonkey uses _content, we support both
+
+  if (linkNode)
+    return [linkNode.href, linkNode];
+
+  // If there is no linkNode, try simple XLink.
+  let href, baseURI;
+  let node = event.target;
+  while (node) {
+    if (node.nodeType == Node.ELEMENT_NODE) {
+      href = node.getAttributeNS("http://www.w3.org/1999/xlink", "href");
+      if (href)
+        baseURI = node.baseURI;
+    }
+    node = node.parentNode;
+  }
+
+  // In case of XLink, we don't return the node we got href from since
+  // callers expect <a>-like elements.
+  return [href ? makeURLAbsolute(baseURI, href) : null, null];
+}
+
+/**
+ * Called whenever the user clicks in the content area.
+ *
+ * @param event
+ *        The click event.
+ * @param isPanelClick
+ *        Whether the event comes from a web panel.
+ * @note default event is prevented if the click is handled.
+ */
+function contentAreaClick(event, isPanelClick)
 {
-         if (!wrapper.href)
-           return true;
-         if (wrapper.getAttribute("onclick"))
-           return true;
-         // javascript links should be executed in the current browser
-         if (wrapper.href.substr(0, 11) === "javascript:")
-           return true;
-         // data links should be executed in the current browser
-         if (wrapper.href.substr(0, 5) === "data:")
+  if (!event.isTrusted || event.getPreventDefault() || event.button == 2)
     return true;
 
-         try {
-           urlSecurityCheck(wrapper.href, wrapper.ownerDocument.nodePrincipal);
-         }
-         catch(ex) {
-           return false;
-         }
-
-         var postData = { };
-         var url = getShortcutOrURI(wrapper.href, postData);
-         if (!url)
-           return true;
-         loadURI(url, null, postData.value, false);
-         event.preventDefault();
-         return false;
-       }
-       else if (linkNode.getAttribute("rel") == "sidebar") {
-         // This is the Opera convention for a special link that - when clicked - allows
-         // you to add a sidebar panel.  We support the Opera convention here.  The link's
-         // title attribute contains the title that should be used for the sidebar panel.
-         PlacesUIUtils.showMinimalAddBookmarkUI(makeURI(wrapper.href),
-                                                wrapper.getAttribute("title"),
-                                                null, null, true, true);
-         event.preventDefault();
-         return false;
-       }
-     }
-     else {
-       handleLinkClick(event, wrapper.href, linkNode);
-     }
-
-     return true;
-   } else {
-     // Try simple XLink
-     var href, realHref, baseURI;
-     linkNode = target;
-     while (linkNode) {
-       if (linkNode.nodeType == Node.ELEMENT_NODE) {
-         wrapper = linkNode;
-
-         realHref = wrapper.getAttributeNS("http://www.w3.org/1999/xlink", "href");
-         if (realHref) {
-           href = realHref;
-           baseURI = wrapper.baseURI
-         }
-       }
-       linkNode = linkNode.parentNode;
-     }
-     if (href) {
-       href = makeURLAbsolute(baseURI, href);
-       handleLinkClick(event, href, null);
-       return true;
-     }
-   }
+  let [href, linkNode] = hrefAndLinkNodeForClickEvent(event);
+  if (!href) {
+    // Not a link, handle middle mouse navigation.
     if (event.button == 1 &&
         gPrefService.getBoolPref("middlemouse.contentLoadURL") &&
         !gPrefService.getBoolPref("general.autoScroll")) {
       middleMousePaste(event);
+      event.preventDefault();
     }
     return true;
   }
 
+  // This code only applies if we have a linkNode (i.e. clicks on real anchor
+  // elements, as opposed to XLink).
+  if (linkNode && event.button == 0 &&
+      !event.ctrlKey && !event.shiftKey && !event.altKey && !event.metaKey) {
+    // A Web panel's links should target the main content area.  Do this
+    // if no modifier keys are down and if there's no target or the target
+    // equals _main (the IE convention) or _content (the Mozilla convention).
+    let target = linkNode.target;
+    let mainTarget = !target || target == "_content" || target  == "_main";
+    if (isPanelClick && mainTarget) {
+      // javascript and data links should be executed in the current browser.
+      if (linkNode.getAttribute("onclick") ||
+          href.substr(0, 11) === "javascript:" ||
+          href.substr(0, 5) === "data:")
+        return true;
+
+      try {
+        urlSecurityCheck(href, linkNode.ownerDocument.nodePrincipal);
+      }
+      catch(ex) {
+        // Prevent loading unsecure destinations.
+        event.preventDefault();
+        return true;
+      }
+
+      let postData = {};
+      let url = getShortcutOrURI(href, postData);
+      if (!url)
+        return true;
+      loadURI(url, null, postData.value, false);
+      event.preventDefault();
+      return true;
+    }
+
+    if (linkNode.getAttribute("rel") == "sidebar") {
+      // This is the Opera convention for a special link that, when clicked,
+      // allows to add a sidebar panel.  The link's title attribute contains
+      // the title that should be used for the sidebar panel.
+      PlacesUIUtils.showMinimalAddBookmarkUI(makeURI(href),
+                                             linkNode.getAttribute("title"),
+                                             null, null, true, true);
+      event.preventDefault();
+      return true;
+    }
+  }
+
+  handleLinkClick(event, href, linkNode);
+  return true;
+}
+
+/**
+ * Handles clicks on links.
+ *
+ * @return true if the click event was handled, false otherwise.
+ */
 function handleLinkClick(event, href, linkNode) {
   if (event.button == 2) // right click
     return false;
@@ -5129,6 +5145,7 @@ function handleLinkClick(event, href, linkNode) {
   if (where == "save") {
     saveURL(href, linkNode ? gatherTextUnder(linkNode) : "", null, true,
             true, doc.documentURIObject);
+    event.preventDefault();
     return true;
   }
 
@@ -5136,7 +5153,7 @@ function handleLinkClick(event, href, linkNode) {
   openLinkIn(href, where, { fromContent: true,
                             referrerURI: doc.documentURIObject,
                             charset: doc.characterSet });
-  event.stopPropagation();
+  event.preventDefault();
   return true;
 }
 

browser/base/content/test/Makefile.in

@@ -221,6 +221,7 @@ _BROWSER_FILES = \
                  browser_aboutHome.js \
                  app_bug575561.html \
                  app_subframe_bug575561.html \
+                 browser_contentAreaClick.js \
                  $(NULL)
 
 # compartment-disabled

browser/base/content/test/browser_contentAreaClick.js

@@ -0,0 +1,296 @@
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Firefox Browser Test Code.
+ *
+ * The Initial Developer of the Original Code is the Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Marco Bonardo <mak77@bonardo.net>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+ 
+/**
+ * Test for bug 549340.
+ * Test for browser.js::contentAreaClick() util.
+ *
+ * The test opens a new browser window, then replaces browser.js methods invoked
+ * by contentAreaClick with a mock function that tracks which methods have been
+ * called.
+ * Each sub-test synthesizes a mouse click event on links injected in content,
+ * the event is collected by a click handler that ensures that contentAreaClick
+ * correctly prevent default events, and follows the correct code path.
+ */
+
+let gTests = [
+
+  {
+    desc: "Simple left click",
+    setup: function() {},
+    clean: function() {},
+    event: {},
+    target: "commonlink",
+    expectedInvokedMethods: [],
+    preventDefault: false,
+  },
+
+  {
+    desc: "Ctrl/Cmd left click",
+    setup: function() {},
+    clean: function() {},
+    event: { ctrlKey: true,
+             metaKey: true },
+    target: "commonlink",
+    expectedInvokedMethods: [ "urlSecurityCheck", "openLinkIn" ],
+    preventDefault: true,
+  },
+
+  // The next test was once handling feedService.forcePreview().  Now it should
+  // just be like Alt click.
+  {
+    desc: "Shift+Alt left click",
+    setup: function() {},
+    clean: function() {},
+    event: { shiftKey: true,
+             altKey: true },
+    target: "commonlink",
+    expectedInvokedMethods: [ "gatherTextUnder", "saveURL" ],
+    preventDefault: true,
+  },
+
+  {
+    desc: "Shift click",
+    setup: function() {},
+    clean: function() {},
+    event: { shiftKey: true },
+    target: "commonlink",
+    expectedInvokedMethods: [ "urlSecurityCheck", "openLinkIn" ],
+    preventDefault: true,
+  },
+
+  {
+    desc: "Alt click",
+    setup: function() {},
+    clean: function() {},
+    event: { altKey: true },
+    target: "commonlink",
+    expectedInvokedMethods: [ "gatherTextUnder", "saveURL" ],
+    preventDefault: true,
+  },
+
+  {
+    desc: "Panel click",
+    setup: function() {},
+    clean: function() {},
+    event: {},
+    target: "panellink",
+    expectedInvokedMethods: [ "urlSecurityCheck", "getShortcutOrURI", "loadURI" ],
+    preventDefault: true,
+  },
+
+  {
+    desc: "Simple middle click opentab",
+    setup: function() {},
+    clean: function() {},
+    event: { button: 1 },
+    target: "commonlink",
+    expectedInvokedMethods: [ "urlSecurityCheck", "openLinkIn" ],
+    preventDefault: true,
+  },
+
+  {
+    desc: "Simple middle click openwin",
+    setup: function() {
+      gPrefService.setBoolPref("browser.tabs.opentabfor.middleclick", false);
+    },
+    clean: function() {
+      try {
+        gPrefService.clearUserPref("browser.tabs.opentabfor.middleclick");
+      } catch(ex) {}
+    },
+    event: { button: 1 },
+    target: "commonlink",
+    expectedInvokedMethods: [ "urlSecurityCheck", "openLinkIn" ],
+    preventDefault: true,
+  },
+
+  {
+    desc: "Middle mouse paste",
+    setup: function() {
+      gPrefService.setBoolPref("middlemouse.contentLoadURL", true);
+      gPrefService.setBoolPref("general.autoScroll", false);
+    },
+    clean: function() {
+      try {
+        gPrefService.clearUserPref("middlemouse.contentLoadURL");
+      } catch(ex) {}
+      try {
+        gPrefService.clearUserPref("general.autoScroll");
+      } catch(ex) {}
+    },
+    event: { button: 1 },
+    target: "emptylink",
+    expectedInvokedMethods: [ "middleMousePaste" ],
+    preventDefault: true,
+  },
+
+];
+
+// Array of method names that will be replaced in the new window.
+let gReplacedMethods = [
+  "middleMousePaste",
+  "urlSecurityCheck",
+  "loadURI",
+  "gatherTextUnder",
+  "saveURL",
+  "openLinkIn",
+  "getShortcutOrURI",
+];
+
+// Reference to the new window.
+let gTestWin = null;
+
+// List of methods invoked by a specific call to contentAreaClick.
+let gInvokedMethods = [];
+
+// The test currently running.
+let gCurrentTest = null;
+
+function test() {
+  waitForExplicitFinish();
+
+  gTestWin = openDialog(location, "", "chrome,all,dialog=no", "about:blank");
+  gTestWin.addEventListener("load", function (event) {
+    info("Window loaded.");
+    gTestWin.removeEventListener("load", arguments.callee, false);
+    waitForFocus(function() {
+      info("Setting up browser...");
+      setupTestBrowserWindow();
+      info("Running tests...");
+      executeSoon(runNextTest);
+    }, gTestWin.content, true);
+  }, false);
+}
+
+// Click handler used to steal click events.
+let gClickHandler = {
+  handleEvent: function (event) {
+    let linkId = event.target.id;
+    is(event.type, "click",
+       gCurrentTest.desc + ":Handler received a click event on " + linkId);
+
+    let isPanelClick = linkId == "panellink";
+    let returnValue = gTestWin.contentAreaClick(event, isPanelClick);
+    let prevent = event.getPreventDefault();
+    is(prevent, gCurrentTest.preventDefault,
+       gCurrentTest.desc + ": event.getPreventDefault() is correct (" + prevent + ")")
+
+    // Check that all required methods have been called.
+    gCurrentTest.expectedInvokedMethods.forEach(function(aExpectedMethodName) {
+      isnot(gInvokedMethods.indexOf(aExpectedMethodName), -1,
+            gCurrentTest.desc + ":" + aExpectedMethodName + " was invoked");
+    });
+    
+    if (gInvokedMethods.length != gCurrentTest.expectedInvokedMethods.length) {
+      is(false, "More than the expected methods have been called");
+      gInvokedMethods.forEach(function (method) info(method + " was invoked"));
+    }
+
+    event.preventDefault();
+    event.stopPropagation();
+
+    executeSoon(runNextTest);
+  }
+}
+
+// Wraps around the methods' replacement mock function.
+function wrapperMethod(aInvokedMethods, aMethodName) {
+  return function () {
+    aInvokedMethods.push(aMethodName);
+    // At least getShortcutOrURI requires to return url that is the first param.
+    return arguments[0];
+  }
+}
+
+function setupTestBrowserWindow() {
+  // Steal click events and don't propagate them.
+  gTestWin.addEventListener("click", gClickHandler, true);
+
+  // Replace methods.
+  gReplacedMethods.forEach(function (aMethodName) {
+    gTestWin["old_" + aMethodName] = gTestWin[aMethodName];
+    gTestWin[aMethodName] = wrapperMethod(gInvokedMethods, aMethodName);
+  });
+
+  // Inject links in content.
+  let doc = gTestWin.content.document;
+  let mainDiv = doc.createElement("div");
+  mainDiv.innerHTML =
+    '<a id="commonlink" href="http://mochi.test/moz/">Common link</a>' +
+    '<a id="panellink" href="http://mochi.test/moz/">Panel link</a>' +
+    '<a id="emptylink">Empty link</a>';
+  doc.body.appendChild(mainDiv);
+}
+
+function runNextTest() {
+  if (gCurrentTest) {
+    info(gCurrentTest.desc + ": cleaning up...")
+    gCurrentTest.clean();
+    gInvokedMethods.length = 0;
+  }
+
+  if (gTests.length > 0) {
+    gCurrentTest = gTests.shift();
+
+    info(gCurrentTest.desc + ": starting...");
+    // Prepare for test.
+    gCurrentTest.setup();
+
+    // Fire click event.
+    let target = gTestWin.content.document.getElementById(gCurrentTest.target);
+    ok(target, gCurrentTest.desc + ": target is valid (" + target.id + ")");
+    EventUtils.synthesizeMouse(target, 2, 2, gCurrentTest.event, gTestWin.content);
+  }
+  else {
+    // No more tests to run.
+    finishTest()
+  }
+}
+
+function finishTest() {
+  info("Restoring browser...");
+  gTestWin.removeEventListener("click", gClickHandler, true);
+
+  // Restore original methods.
+  gReplacedMethods.forEach(function (aMethodName) {
+    gTestWin[aMethodName] = gTestWin["old_" + aMethodName];
+    delete gTestWin["old_" + aMethodName];
+  });
+
+  gTestWin.close();
+  finish();
+}