Bug 656379 part 3. Set :hover and :active state for labeled elements when their label has that state. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 24 May 2011 20:18:40 -0400
changeset 70142 9ab424c86a26ff417a72715876df7dd647118cfe
parent 70141 2e27b606caed11b51f680c09da8c3dfe3cdc1d2d
child 70143 f6fbc63f12a1ef765602c1208bf5bc35f7cae585
push id20200
push usermlamouri@mozilla.com
push dateWed, 25 May 2011 08:04:31 +0000
treeherdermozilla-central@b909ecef2c15 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs656379
milestone7.0a1
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 656379 part 3. Set :hover and :active state for labeled elements when their label has that state. r=dbaron
content/events/src/nsEventStateManager.cpp
content/events/src/nsEventStateManager.h
content/events/test/Makefile.in
content/events/test/test_bug656379-1.html
content/events/test/test_bug656379-2.html
widget/src/xpwidgets/nsNativeTheme.cpp
--- a/content/events/src/nsEventStateManager.cpp
+++ b/content/events/src/nsEventStateManager.cpp
@@ -148,16 +148,17 @@
 #include "nsContentAreaDragDrop.h"
 #ifdef MOZ_XUL
 #include "nsTreeBodyFrame.h"
 #endif
 #include "nsIController.h"
 #include "nsICommandParams.h"
 #include "mozilla/Services.h"
 #include "mozAutoDocUpdate.h"
+#include "nsHTMLLabelElement.h"
 
 #ifdef XP_MACOSX
 #import <ApplicationServices/ApplicationServices.h>
 #endif
 
 using namespace mozilla::dom;
 
 //#define DEBUG_DOCSHELL_FOCUS
@@ -4222,45 +4223,36 @@ nsEventStateManager::GetEventTargetConte
   // event target in the PresShell.
   if (!content && mCurrentTarget) {
     mCurrentTarget->GetContentForEvent(mPresContext, aEvent, &content);
   }
 
   return content;
 }
 
-static already_AddRefed<nsIContent>
-GetLabelTarget(nsIContent* aLabel)
+static Element*
+GetLabelTarget(nsIContent* aPossibleLabel)
 {
-  nsCOMPtr<nsIDOMHTMLLabelElement> label = do_QueryInterface(aLabel);
+  nsHTMLLabelElement* label = nsHTMLLabelElement::FromContent(aPossibleLabel);
   if (!label)
     return nsnull;
 
-  nsCOMPtr<nsIDOMHTMLElement> target;
-  label->GetControl(getter_AddRefs(target));
-  nsIContent* targetContent = nsnull;
-  if (target) {
-    CallQueryInterface(target, &targetContent);
-  }
-  return targetContent;
+  return label->GetLabeledElement();
 }
 
 static bool
-IsAncestorOf(nsIContent* aPossibleAncestor, nsIContent* aPossibleDescendant,
-             PRBool aFollowLabels)
+IsAncestorOf(nsIContent* aPossibleAncestor, nsIContent* aPossibleDescendant)
 {
   for (; aPossibleDescendant; aPossibleDescendant = aPossibleDescendant->GetParent()) {
     if (aPossibleAncestor == aPossibleDescendant)
       return true;
 
-    if (aFollowLabels) {
-      nsCOMPtr<nsIContent> labelTarget = GetLabelTarget(aPossibleDescendant);
-      if (labelTarget == aPossibleAncestor)
-        return true;
-    }
+    Element* labelTarget = GetLabelTarget(aPossibleDescendant);
+    if (labelTarget == aPossibleAncestor)
+      return true;
   }
   return false;
 }
 
 static bool
 ShouldShowFocusRing(nsIContent* aContent)
 {
   nsIDocument* doc = aContent->GetOwnerDoc();
@@ -4268,24 +4260,24 @@ ShouldShowFocusRing(nsIContent* aContent
     nsPIDOMWindow* window = doc->GetWindow();
     return window && window->ShouldShowFocusRing();
   }
 
   return false;
 }
 
 nsEventStates
-nsEventStateManager::GetContentState(nsIContent *aContent, PRBool aFollowLabels)
+nsEventStateManager::GetContentState(nsIContent *aContent)
 {
   nsEventStates state = aContent->IntrinsicState();
 
-  if (IsAncestorOf(aContent, mActiveContent, aFollowLabels)) {
+  if (IsAncestorOf(aContent, mActiveContent)) {
     state |= NS_EVENT_STATE_ACTIVE;
   }
-  if (IsAncestorOf(aContent, mHoverContent, aFollowLabels)) {
+  if (IsAncestorOf(aContent, mHoverContent)) {
     state |= NS_EVENT_STATE_HOVER;
   }
 
   nsFocusManager* fm = nsFocusManager::GetFocusManager();
   nsIContent* focusedContent = fm ? fm->GetFocusedContent() : nsnull;
   if (aContent == focusedContent) {
     state |= NS_EVENT_STATE_FOCUS;
 
@@ -4347,17 +4339,17 @@ static nsIContent* FindCommonAncestor(ns
 }
 
 static void
 NotifyAncestors(nsIDocument* aDocument, nsIContent* aStartNode,
                 nsIContent* aStopBefore, nsEventStates aState)
 {
   while (aStartNode && aStartNode != aStopBefore) {
     aDocument->ContentStateChanged(aStartNode, aState);
-    nsCOMPtr<nsIContent> labelTarget = GetLabelTarget(aStartNode);
+    Element* labelTarget = GetLabelTarget(aStartNode);
     if (labelTarget) {
       aDocument->ContentStateChanged(labelTarget, aState);
     }
     aStartNode = aStartNode->GetParent();
   }
 }
 
 PRBool
--- a/content/events/src/nsEventStateManager.h
+++ b/content/events/src/nsEventStateManager.h
@@ -117,25 +117,19 @@ public:
   void ClearFrameRefs(nsIFrame* aFrame);
 
   nsIFrame* GetEventTarget();
   already_AddRefed<nsIContent> GetEventTargetContent(nsEvent* aEvent);
 
   /**
    * Returns the content state of aContent.
    * @param aContent      The control whose state is requested.
-   * @param aFollowLabels Whether to reflect a label's content state on its
-   *                      associated control. If aFollowLabels is true and
-   *                      aContent is a control which has a label that has the 
-   *                      hover or active content state set, GetContentState
-   *                      will pretend that those states are also set on aContent.
    * @return              The content state.
    */
-  virtual nsEventStates GetContentState(nsIContent *aContent,
-                                        PRBool aFollowLabels = PR_FALSE);
+  virtual nsEventStates GetContentState(nsIContent *aContent);
 
   /**
    * Notify that the given NS_EVENT_STATE_* bit has changed for this content.
    * @param aContent Content which has changed states
    * @param aState   Corresponding state flags such as NS_EVENT_STATE_FOCUS
    * @return  Whether the content was able to change all states. Returns PR_FALSE
    *                  if a resulting DOM event causes the content node passed in
    *                  to not change states. Note, the frame for the content may
--- a/content/events/test/Makefile.in
+++ b/content/events/test/Makefile.in
@@ -99,16 +99,18 @@ include $(topsrcdir)/config/rules.mk
 		test_bug605242.html \
 		test_bug613634.html \
 		test_bug607464.html \
 		test_bug624127.html \
 		test_bug650493.html \
 		test_bug641477.html \
 		test_bug648573.html \
 		test_bug615597.html \
+		test_bug656379-1.html \
+		test_bug656379-2.html \
 		test_bug656954.html \
 		$(NULL)
 
 #bug 585630
 ifneq (mobile,$(MOZ_BUILD_APP))
 _TEST_FILES += \
 		test_dragstart.html \
 		$(NULL)
copy from content/events/test/test_bug426082.html
copy to content/events/test/test_bug656379-1.html
--- a/content/events/test/test_bug426082.html
+++ b/content/events/test/test_bug656379-1.html
@@ -1,24 +1,44 @@
 <!DOCTYPE HTML>
 <html>
 <!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=426082
+https://bugzilla.mozilla.org/show_bug.cgi?id=656379
 -->
 <head>
-  <title>Test for Bug 426082</title>
+  <title>Test for Bug 656379</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>
   <script type="application/javascript" src="/tests/SimpleTest/WindowSnapshot.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <style>
     canvas {
       display: none;
     }
+    input[type=button] {
+      -moz-appearance: none;
+      padding: 0;
+      border: none;
+      color: black;
+      background: white;
+    }
+    input[type=button]::-moz-focus-inner { border: none; }
+
+    /* Make sure that normal, focused, hover+active, focused+hover+active
+       buttons all have different styles so that the test keeps moving along. */
+    input[type=button]:hover:active {
+      background: red;
+    }
+    input[type=button]:focus {
+      background: green;
+    }
+    input[type=button]:focus:hover:active {
+      background: purple;
+    }
   </style>
 </head>
 <body onload="runTests()">
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=426082">Mozilla Bug 426082</a>
 <p id="display"></p>
 <div id="content" style="display: none">
   
 </div>
new file mode 100644
--- /dev/null
+++ b/content/events/test/test_bug656379-2.html
@@ -0,0 +1,63 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=656379
+-->
+<head>
+  <title>Test for Bug 656379</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"/>
+  <style>
+    button:hover { color: green; }
+  </style>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=656379">Mozilla Bug 656379</a>
+<p id="display">
+  <label for="button1" id="label1">Label 1</label>
+  <button id="button1">Button 1</button>
+  <label><span id="label2">Label 2</span><button id="button2">Button 2</button></label>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript;version=1.8">
+
+/** Test for Bug 656379 **/
+SimpleTest.waitForExplicitFinish();
+function tests() {
+  synthesizeMouseAtCenter($("label1"), { type: "mousemove" });
+  yield;
+  is($("button1").mozMatchesSelector(":hover"), true,
+     "Button 1 should be hovered");
+  is($("button2").mozMatchesSelector(":hover"), false,
+     "Button 2 should not be hovered");
+  synthesizeMouseAtCenter($("label2"), { type: "mousemove" });
+  yield;
+  is($("button1").mozMatchesSelector(":hover"), false,
+     "Button 1 should not be hovered");
+  is($("button2").mozMatchesSelector(":hover"), true,
+     "Button 2 should be hovered");
+  SimpleTest.finish();
+}
+
+function executeTests() {
+  var testYielder = tests();
+  function execNext() {
+    try {
+      testYielder.next();
+      SimpleTest.executeSoon(execNext);
+    } catch(e) {}
+  }
+  execNext();
+}
+
+SimpleTest.waitForFocus(executeTests);
+
+</script>
+</pre>
+</body>
+</html>
--- a/widget/src/xpwidgets/nsNativeTheme.cpp
+++ b/widget/src/xpwidgets/nsNativeTheme.cpp
@@ -88,17 +88,17 @@ nsNativeTheme::GetContentState(nsIFrame*
   if (!aFrame->GetContent())
     return nsEventStates();
 
   nsIPresShell *shell = GetPresShell(aFrame);
   if (!shell)
     return nsEventStates();
 
   nsEventStateManager* esm = shell->GetPresContext()->EventStateManager();
-  nsEventStates flags = esm->GetContentState(aFrame->GetContent(), PR_TRUE);
+  nsEventStates flags = esm->GetContentState(aFrame->GetContent());
   
   if (isXULCheckboxRadio && aWidgetType == NS_THEME_RADIO) {
     if (IsFocused(aFrame))
       flags |= NS_EVENT_STATE_FOCUS;
   }
 
   // On Windows and Mac, only draw focus rings if they should be shown. This
   // means that focus rings are only shown once the keyboard has been used to