Bug 331959 and bug 127903. Make situations in which an anchor or submit control is nested inside another anchor and the inner thing is clicked trigger the inner thing, not both. r=smaug,gavin a=blocker
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 27 Dec 2010 14:42:10 -0600
changeset 59693 b7acaa08a9c5cf43831ce524e8d8c8337c3df7ca
parent 59692 2382fb87cbe28d7d0ee7f3dff7d0bd88aa58b013
child 59694 93a8fb0292bab50e6b1816ef49d531531e48131f
push idunknown
push userunknown
push dateunknown
reviewerssmaug, gavin, blocker
bugs331959, 127903
milestone2.0b9pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 331959 and bug 127903. Make situations in which an anchor or submit control is nested inside another anchor and the inner thing is clicked trigger the inner thing, not both. r=smaug,gavin a=blocker
browser/base/content/browser.js
browser/base/content/nsContextMenu.js
content/base/src/nsGenericElement.cpp
content/base/test/Makefile.in
content/base/test/test_bug331959.html
content/html/content/src/nsHTMLButtonElement.cpp
content/html/content/src/nsHTMLInputElement.cpp
widget/public/nsGUIEvent.h
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -5031,51 +5031,33 @@ function asyncOpenWebPanel(event)
  *
  * @note linkNode will be null if the click wasn't on an anchor
  *       element (or XLink).
  */
 function hrefAndLinkNodeForClickEvent(event)
 {
   function isHTMLLink(aNode)
   {
-    return aNode instanceof HTMLAnchorElement ||
-           aNode instanceof HTMLAreaElement ||
-           aNode instanceof HTMLLinkElement;
-  }
-
-  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 {
-    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 (node && node.hasAttribute("href"))
-      linkNode = node;
-  }
-
-  if (linkNode)
-    return [linkNode.href, linkNode];
+    // Be consistent with what nsContextMenu.js does.
+    return ((aNode instanceof HTMLAnchorElement && aNode.href) ||
+            (aNode instanceof HTMLAreaElement && aNode.href) ||
+            aNode instanceof HTMLLinkElement);
+  }
+
+  let node = event.target;
+  while (node && !isHTMLLink(node)) {
+    node = node.parentNode;
+  }
+
+  if (node)
+    return [node.href, node];
 
   // If there is no linkNode, try simple XLink.
   let href, baseURI;
-  let node = event.target;
+  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;
   }
--- a/browser/base/content/nsContextMenu.js
+++ b/browser/base/content/nsContextMenu.js
@@ -536,42 +536,28 @@ nsContextMenu.prototype = {
     // Second, bubble out, looking for items of interest that can have childen.
     // Always pick the innermost link, background image, etc.
     const XMLNS = "http://www.w3.org/XML/1998/namespace";
     var elem = this.target;
     while (elem) {
       if (elem.nodeType == Node.ELEMENT_NODE) {
         // Link?
         if (!this.onLink &&
+            // Be consistent with what hrefAndLinkNodeForClickEvent
+            // does in browser.js
              ((elem instanceof HTMLAnchorElement && elem.href) ||
               (elem instanceof HTMLAreaElement && elem.href) ||
               elem instanceof HTMLLinkElement ||
               elem.getAttributeNS("http://www.w3.org/1999/xlink", "type") == "simple")) {
 
           // Target is a link or a descendant of a link.
           this.onLink = true;
 
-          // 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 realLink = elem;
-          var parent = elem;
-          while ((parent = parent.parentNode) &&
-                 (parent.nodeType == Node.ELEMENT_NODE)) {
-            try {
-              if ((parent instanceof HTMLAnchorElement && parent.href) ||
-                  (parent instanceof HTMLAreaElement && parent.href) ||
-                  parent instanceof HTMLLinkElement ||
-                  parent.getAttributeNS("http://www.w3.org/1999/xlink", "type") == "simple")
-                realLink = parent;
-            } catch (e) { }
-          }
-
           // Remember corresponding element.
-          this.link = realLink;
+          this.link = elem;
           this.linkURL = this.getLinkURL();
           this.linkURI = this.getLinkURI();
           this.linkProtocol = this.getLinkProtocol();
           this.onMailtoLink = (this.linkProtocol == "mailto");
           this.onSaveableLink = this.isLinkSaveable( this.link );
         }
 
         // Background image?  Don't bother if we've already found a
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -5203,17 +5203,18 @@ nsGenericElement::CreateSlots()
 }
 
 PRBool
 nsGenericElement::CheckHandleEventForLinksPrecondition(nsEventChainVisitor& aVisitor,
                                                        nsIURI** aURI) const
 {
   if (aVisitor.mEventStatus == nsEventStatus_eConsumeNoDefault ||
       !NS_IS_TRUSTED_EVENT(aVisitor.mEvent) ||
-      !aVisitor.mPresContext) {
+      !aVisitor.mPresContext ||
+      (aVisitor.mEvent->flags & NS_EVENT_FLAG_PREVENT_ANCHOR_ACTIONS)) {
     return PR_FALSE;
   }
 
   // Make sure we actually are a link
   return IsLink(aURI);
 }
 
 nsresult
@@ -5248,24 +5249,29 @@ nsGenericElement::PreHandleEventForLinks
     // FALL THROUGH
   case NS_FOCUS_CONTENT:
     if (aVisitor.mEvent->eventStructType != NS_FOCUS_EVENT ||
         !static_cast<nsFocusEvent*>(aVisitor.mEvent)->isRefocus) {
       nsAutoString target;
       GetLinkTarget(target);
       nsContentUtils::TriggerLink(this, aVisitor.mPresContext, absURI, target,
                                   PR_FALSE, PR_TRUE);
+      // Make sure any ancestor links don't also TriggerLink
+      aVisitor.mEvent->flags |= NS_EVENT_FLAG_PREVENT_ANCHOR_ACTIONS;
     }
     break;
 
   case NS_MOUSE_EXIT_SYNTH:
     aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
     // FALL THROUGH
   case NS_BLUR_CONTENT:
     rv = LeaveLink(aVisitor.mPresContext);
+    if (NS_SUCCEEDED(rv)) {
+      aVisitor.mEvent->flags |= NS_EVENT_FLAG_PREVENT_ANCHOR_ACTIONS;
+    }
     break;
 
   default:
     // switch not in sync with the optimization switch earlier in this function
     NS_NOTREACHED("switch statements not in sync");
     return NS_ERROR_UNEXPECTED;
   }
 
@@ -5302,16 +5308,17 @@ nsGenericElement::PostHandleEventForLink
           static_cast<nsMouseEvent*>(aVisitor.mEvent)->button ==
           nsMouseEvent::eLeftButton) {
         // don't make the link grab the focus if there is no link handler
         nsILinkHandler *handler = aVisitor.mPresContext->GetLinkHandler();
         nsIDocument *document = GetCurrentDoc();
         if (handler && document) {
           nsIFocusManager* fm = nsFocusManager::GetFocusManager();
           if (fm) {
+            aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
             nsCOMPtr<nsIDOMElement> elem = do_QueryInterface(this);
             fm->SetFocus(elem, nsIFocusManager::FLAG_BYMOUSE |
                                nsIFocusManager::FLAG_NOSCROLL);
           }
 
           nsIEventStateManager* esm =
             aVisitor.mPresContext->EventStateManager();
           nsEventStateManager::SetActiveManager(
@@ -5333,26 +5340,35 @@ nsGenericElement::PostHandleEventForLink
       nsCOMPtr<nsIPresShell> shell = aVisitor.mPresContext->GetPresShell();
       if (shell) {
         // single-click
         nsEventStatus status = nsEventStatus_eIgnore;
         nsUIEvent actEvent(NS_IS_TRUSTED_EVENT(aVisitor.mEvent),
                            NS_UI_ACTIVATE, 1);
 
         rv = shell->HandleDOMEventWithTarget(this, &actEvent, &status);
+        if (NS_SUCCEEDED(rv)) {
+          aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+        }
       }
     }
     break;
 
   case NS_UI_ACTIVATE:
     {
-      nsAutoString target;
-      GetLinkTarget(target);
-      nsContentUtils::TriggerLink(this, aVisitor.mPresContext, absURI, target,
-                                  PR_TRUE, PR_TRUE);
+      nsCOMPtr<nsIContent> targetContent;
+      aVisitor.mPresContext->EventStateManager()->
+        GetEventTargetContent(aVisitor.mEvent, getter_AddRefs(targetContent));
+      if (targetContent == this) {
+        nsAutoString target;
+        GetLinkTarget(target);
+        nsContentUtils::TriggerLink(this, aVisitor.mPresContext, absURI, target,
+                                    PR_TRUE, PR_TRUE);
+        aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+      }
     }
     break;
 
   case NS_KEY_PRESS:
     {
       if (aVisitor.mEvent->eventStructType == NS_KEY_EVENT) {
         nsKeyEvent* keyEvent = static_cast<nsKeyEvent*>(aVisitor.mEvent);
         if (keyEvent->keyCode == NS_VK_RETURN) {
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -77,16 +77,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug276037-1.html \
 		test_bug276037-2.xhtml \
 		test_bug298064.html \
 		bug298064-subframe.html \
 		test_xhr_forbidden_headers.html \
 		test_bug311681.xml \
 		test_bug322317.html \
 		test_bug330925.xhtml \
+		test_bug331959.html \
 		test_bug333673.html \
 		test_bug337631.html \
 		test_bug338541.xhtml \
 		test_bug338679.html \
 		test_bug339494.html \
 		test_bug339494.xhtml \
 		test_bug339494.xul \
 		test_bug340571.html \
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug331959.html
@@ -0,0 +1,143 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=331959
+-->
+<head>
+  <title>Test for Bug 331959</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=331959">Mozilla Bug 331959</a>
+<p id="display">
+  <iframe id="link-in-link-mouse"></iframe>
+  <iframe id="link-in-link-keyboard"></iframe>
+  <iframe id="input-in-link-mouse"></iframe>
+  <iframe id="input-in-link-keyboard"></iframe>
+  <iframe id="button-in-link-mouse"></iframe>
+  <iframe id="button-in-link-keyboard"></iframe>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 331959 **/
+SimpleTest.waitForExplicitFinish();
+
+const FAILURL = "data:text/plain,FAIL";
+const PASSURL = "data:text/plain,PASS";
+
+var currentTest = 0;
+var tests = [ testLinkInLinkMouse, testLinkInLinkKeyboard,
+              testInputInLinkMouse, testInputInLinkKeyboard,
+              testButtonInLinkMouse, testButtonInLinkKeyboard ];
+function doNextTest() {
+  if (currentTest == tests.length) {
+    SimpleTest.finish();
+  } else {
+    tests[currentTest++]();
+  }
+}
+
+function generateLinkInLink(id, desc) {
+  var doc = $(id).contentDocument;
+  var outerA = doc.createElement("a");
+  var innerA = doc.createElement("a");
+  outerA.id = "outer";
+  innerA.id = "inner";
+  innerA.href = PASSURL;
+  outerA.href = FAILURL;
+  innerA.appendChild(doc.createTextNode("Text"));
+  outerA.appendChild(innerA);
+  doc.body.appendChild(outerA);
+  $(id).onload = function() {
+    is(this.contentDocument.documentElement.textContent, "PASS", desc);
+    doNextTest();
+  };
+  return [innerA, $(id).contentWindow];
+}
+
+function testLinkInLinkMouse() {
+  var [innerA, testWin] =
+    generateLinkInLink("link-in-link-mouse",
+                       "Clicking an inner link should load the inner link");
+  synthesizeMouseAtCenter(innerA, {}, testWin);
+}
+
+function testLinkInLinkKeyboard() {
+  var [innerA, testWin] =
+    generateLinkInLink("link-in-link-keyboard",
+                       "Hitting enter on an inner link should load the inner link");
+  innerA.focus();
+  synthesizeKey("VK_RETURN", {}, testWin);
+}
+
+function generateInputInLink(id, desc) {
+  var doc = $(id).contentDocument;
+  doc.body.innerHTML =
+    "<form action='" + PASSURL + "'><a href='" + FAILURL +
+    "'><input type='submit' id='submit'>";
+  $(id).onload = function() {
+    is(this.contentDocument.documentElement.textContent, "PASS?", desc);
+    doNextTest();
+  };
+  var input = doc.getElementById("submit");
+  doc.body.offsetWidth;
+  return [input, $(id).contentWindow];
+}
+
+function testInputInLinkMouse() {
+  var [input, testWin] =
+    generateInputInLink("input-in-link-mouse",
+                        "Clicking an submit input inside an anchor should submit the form");
+  synthesizeMouseAtCenter(input, {}, testWin);
+}
+
+function testInputInLinkKeyboard() {
+  var [input, testWin] =
+    generateInputInLink("input-in-link-keyboard",
+                        "Return on submit input inside an anchor should submit the form");
+  input.focus();
+  synthesizeKey("VK_RETURN", {}, testWin);
+}
+
+function generateButtonInLink(id, desc) {
+  var doc = $(id).contentDocument;
+  doc.body.innerHTML =
+    "<form action='" + PASSURL + "'><a href='" + FAILURL +
+    "'><button type='submit' id='submit'>Submit</button>";
+  $(id).onload = function() {
+    is(this.contentDocument.documentElement.textContent, "PASS?", desc);
+    doNextTest();
+  };
+  var button = doc.getElementById("submit");
+  return [button, $(id).contentWindow];
+}
+
+function testButtonInLinkMouse() {
+  var [button, testWin] =
+    generateButtonInLink("button-in-link-mouse",
+                        "Clicking an submit button inside an anchor should submit the form");
+  synthesizeMouseAtCenter(button, {}, testWin);
+}
+
+function testButtonInLinkKeyboard() {
+  var [button, testWin] =
+    generateButtonInLink("button-in-link-keyboard",
+                        "Return on submit button inside an anchor should submit the form");
+  button.focus();
+  synthesizeKey("VK_RETURN", {}, testWin);
+}
+
+// We need focus to handle clicks properly
+SimpleTest.waitForFocus(doNextTest);
+
+</script>
+</pre>
+</body>
+</html>
--- a/content/html/content/src/nsHTMLButtonElement.cpp
+++ b/content/html/content/src/nsHTMLButtonElement.cpp
@@ -422,16 +422,17 @@ nsHTMLButtonElement::PostHandleEvent(nsE
 
             nsMouseEvent event(NS_IS_TRUSTED_EVENT(aVisitor.mEvent),
                                NS_MOUSE_CLICK, nsnull,
                                nsMouseEvent::eReal);
             event.inputSource = nsIDOMNSMouseEvent::MOZ_SOURCE_KEYBOARD;
             nsEventDispatcher::Dispatch(static_cast<nsIContent*>(this),
                                         aVisitor.mPresContext, &event, nsnull,
                                         &status);
+            aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
           }
         }
         break;// NS_KEY_PRESS
 
       case NS_MOUSE_BUTTON_DOWN:
         {
           if (aVisitor.mEvent->eventStructType == NS_MOUSE_EVENT) {
             if (static_cast<nsMouseEvent*>(aVisitor.mEvent)->button ==
@@ -520,16 +521,17 @@ nsHTMLButtonElement::PostHandleEvent(nsE
                           // make sure formnovalidate is used only if it's a submit control.
                           HasAttr(kNameSpaceID_None, nsGkAtoms::formnovalidate) ||
                           mForm->CheckValidFormSubmission())) {
           // TODO: removing this code and have the submit event sent by the form
           // see bug 592124.
           // Hold a strong ref while dispatching
           nsRefPtr<nsHTMLFormElement> form(mForm);
           presShell->HandleDOMEventWithTarget(mForm, &event, &status);
+          aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
         }
       }
     }
   } else if ((aVisitor.mItemFlags & NS_IN_SUBMIT_CLICK) && mForm) {
     // Tell the form to flush a possible pending submission.
     // the reason is that the script returned false (the event was
     // not ignored) so if there is a stored submission, it needs to
     // be submitted immediatelly.
--- a/content/html/content/src/nsHTMLInputElement.cpp
+++ b/content/html/content/src/nsHTMLInputElement.cpp
@@ -2302,16 +2302,17 @@ nsHTMLInputElement::PostHandleEvent(nsEv
                 nsMouseEvent event(NS_IS_TRUSTED_EVENT(aVisitor.mEvent),
                                    NS_MOUSE_CLICK, nsnull, nsMouseEvent::eReal);
                 event.inputSource = nsIDOMNSMouseEvent::MOZ_SOURCE_KEYBOARD;
                 nsEventStatus status = nsEventStatus_eIgnore;
 
                 nsEventDispatcher::Dispatch(static_cast<nsIContent*>(this),
                                             aVisitor.mPresContext, &event,
                                             nsnull, &status);
+                aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
               } // case
             } // switch
           }
           if (aVisitor.mEvent->message == NS_KEY_PRESS &&
               mType == NS_FORM_INPUT_RADIO && !keyEvent->isAlt &&
               !keyEvent->isControl && !keyEvent->isMeta) {
             PRBool isMovingBack = PR_FALSE;
             switch (keyEvent->keyCode) {
@@ -2468,16 +2469,17 @@ nsHTMLInputElement::PostHandleEvent(nsEv
                               mForm->HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate) ||
                               // We know the element is a submit control, if this check is moved,
                               // make sure formnovalidate is used only if it's a submit control.
                               HasAttr(kNameSpaceID_None, nsGkAtoms::formnovalidate) ||
                               mForm->CheckValidFormSubmission())) {
               // Hold a strong ref while dispatching
               nsRefPtr<nsHTMLFormElement> form(mForm);
               presShell->HandleDOMEventWithTarget(mForm, &event, &status);
+              aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
             }
           }
           break;
 
         default:
           break;
         } //switch 
       } //click or outer activate event
--- a/widget/public/nsGUIEvent.h
+++ b/widget/public/nsGUIEvent.h
@@ -153,16 +153,18 @@ class nsHashKey;
 
 // A flag for drag&drop handling.
 #define NS_EVENT_FLAG_NO_DEFAULT_CALLED_IN_CONTENT 0x4000
 
 #define NS_PRIV_EVENT_UNTRUSTED_PERMITTED 0x8000
 
 #define NS_EVENT_FLAG_EXCEPTION_THROWN    0x10000
 
+#define NS_EVENT_FLAG_PREVENT_ANCHOR_ACTIONS 0x10000
+
 #define NS_EVENT_CAPTURE_MASK             (~(NS_EVENT_FLAG_BUBBLE | NS_EVENT_FLAG_NO_CONTENT_DISPATCH))
 #define NS_EVENT_BUBBLE_MASK              (~(NS_EVENT_FLAG_CAPTURE | NS_EVENT_FLAG_NO_CONTENT_DISPATCH))
 
 #define NS_EVENT_TYPE_NULL                   0
 
 /**
  * GUI MESSAGES
  */