Bug 599328 - Clicking a label shouldn't draw a focus ring if clicking the control wouldn't have. r=enndeakin

--HG--
extra : rebase_source : 6126db042e918a329739c142330e4ac870bff0f5

dom/html/HTMLLabelElement.cpp

@@ -169,8 +169,14 @@ HTMLLabelElement::PostHandleEvent(EventChainPostVisitor& aVisitor)
               // be selected only when focused via a key or when the navigation
               // flag is used and we want to select the text on label clicks as
               // well.
+              // If the label has been clicked by the user, we also want to
+              // pass FLAG_BYMOUSE so that we get correct focus ring behavior,
+              // but we don't want to pass FLAG_BYMOUSE if this click event was
+              // caused by the user pressing an accesskey.
               nsCOMPtr<nsIDOMElement> elem = do_QueryInterface(content);
-              fm->SetFocus(elem, nsIFocusManager::FLAG_BYMOVEFOCUS);
+              bool byMouse = (mouseEvent->inputSource != nsIDOMMouseEvent::MOZ_SOURCE_KEYBOARD);
+              fm->SetFocus(elem, nsIFocusManager::FLAG_BYMOVEFOCUS |
+                                 (byMouse ? nsIFocusManager::FLAG_BYMOUSE : 0));
             }
           }
           // Dispatch a new click event to |content|

dom/tests/mochitest/chrome/window_focus.xul

@@ -147,10 +147,13 @@ function expectFocusShift(callback, expectedWindow, expectedElement, focusChange
     // for this test which fires a mouse event on a label, the document will
     // be focused first and then the label code will focus the related
     // control. This doesn't result in different focus events, but a command
-    // update will occur for the document and then a secon command update will
-    // occur when the control is focused.
-    if (testid == "mouse on html label with content inside")
+    // update will occur for the document and then a second command update will
+    // occur when the control is focused. However, this will only happen on
+    // platforms or controls where mouse clicks cause trigger focus.
+    if (testid == "mouse on html label with content inside" &&
+        mouseWillTriggerFocus(expectedElement)) {
       expectedEvents += " commandupdate: cu";
+    }
 
     if (expectedElement &&
         (!gNewExpectedWindow || gNewExpectedWindow.document.documentElement != expectedElement)) {
@@ -278,25 +281,36 @@ function getTopWindow(win)
              getInterface(Components.interfaces.nsIDOMWindow);
 }
 
+function mouseWillTriggerFocus(element)
+{
+  if (!element) {
+    return false;
+  }
+
+  if (navigator.platform.indexOf("Mac") != 0) {
+    return true;
+  }
+
+  if (element.namespaceURI == "http://www.w3.org/1999/xhtml") {
+    // links are special. They can be focused but show no focus ring
+    if (element.localName == "a" || element.localName == "div" ||
+        element.localName == "select" ||
+        element.localName == "input" && (element.type == "text" ||
+                                         element.type == "password")) {
+      return true;
+    }
+  } else if (element.localName == "listbox") {
+    return true;
+  }
+
+  return false;
+}
+
 function mouseOnElement(element, expectedElement, focusChanged, testid)
 {
   var expectedWindow = (element.ownerDocument.defaultView == gChildWindow) ? gChildWindow : window;
   // on Mac, form elements are not focused when clicking, except for lists and textboxes.
-  var noFocusOnMouse = (navigator.platform.indexOf("Mac") == 0);
-  if (noFocusOnMouse) {
-    if (element.namespaceURI == "http://www.w3.org/1999/xhtml") {
-      // links are special. They can be focused but show no focus ring
-      if (element.localName == "a" || element.localName == "div" ||
-          element.localName == "select" ||
-          element.localName == "input" && (element.type == "text" ||
-                                           element.type == "password")) {
-        noFocusOnMouse = false;
-      }
-    }
-    else if (element.localName == "listbox") {
-      noFocusOnMouse = false;
-    }
-  }
+  var noFocusOnMouse = !mouseWillTriggerFocus(element)
 
   if (noFocusOnMouse) {
     // no focus so the last focus method will be 0
@@ -552,11 +566,9 @@ function startTest()
   }
 
   // clicking on the labels
-  gLastFocusMethod = fm.FLAG_BYMOVEFOCUS;
-  expectFocusShift(function () synthesizeMouse(getById("ad"), 2, 2, { }, gChildWindow),
-                   null, getById("t29"), true, "mouse on html label with content inside");
-  expectFocusShift(function () synthesizeMouse(getById("ag"), 2, 2, { }, gChildWindow),
-                   null, getById("n6"), true, "mouse on html label with for attribute");
+  gLastFocusMethod = fm.FLAG_BYMOVEFOCUS | fm.FLAG_BYMOUSE;
+  mouseOnElement(getById("ad"), getById("t29"), true, "mouse on html label with content inside");
+  mouseOnElement(getById("ag"), getById("n6"), true, "mouse on html label with for attribute");
   gLastFocusMethod = 0;
   expectFocusShift(function () synthesizeMouse(getById("aj"), 2, 2, { }),
                    null, getById("o9"), true, "mouse on xul label with content inside");