Bug 1249556 - Implement toggling details by keyboard. r=smaug
authorTing-Yu Lin <tlin@mozilla.com>
Sat, 19 Mar 2016 20:37:09 +0800
changeset 289553 a2fe8535e63bcddaa25f838b8f7de41a7ad2a0cb
parent 289552 1e26562f4f3673214e34c4733bcce3e342c81dbc
child 289554 c74f71e1ba625b9ad16b6868187be17595e249ee
push id18283
push usercbook@mozilla.com
push dateMon, 21 Mar 2016 15:02:44 +0000
treeherderfx-team@af22a2e633c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1249556
milestone48.0a1
Bug 1249556 - Implement toggling details by keyboard. r=smaug The user can switch to the main <summary> by tab key, and toggle the <details> by either 'space' key or 'enter' key. 'return' key is handled with 'keypress', and the 'space' key is handled with 'keyup' like the HTMLInputElement. MozReview-Commit-ID: HE6IduUGCpj
dom/html/HTMLButtonElement.cpp
dom/html/HTMLInputElement.cpp
dom/html/HTMLSummaryElement.cpp
dom/html/HTMLSummaryElement.h
dom/html/nsGenericHTMLElement.cpp
dom/html/nsGenericHTMLElement.h
layout/reftests/details-summary/key-enter-open-second-summary.html
layout/reftests/details-summary/key-enter-prevent-default.html
layout/reftests/details-summary/key-enter-single-summary.html
layout/reftests/details-summary/key-space-single-summary.html
layout/reftests/details-summary/reftest.list
--- a/dom/html/HTMLButtonElement.cpp
+++ b/dom/html/HTMLButtonElement.cpp
@@ -287,25 +287,18 @@ HTMLButtonElement::PostHandleEvent(Event
         {
           // For backwards compat, trigger buttons with space or enter
           // (bug 25300)
           WidgetKeyboardEvent* keyEvent = aVisitor.mEvent->AsKeyboardEvent();
           if ((keyEvent->keyCode == NS_VK_RETURN &&
                eKeyPress == aVisitor.mEvent->mMessage) ||
               (keyEvent->keyCode == NS_VK_SPACE &&
                eKeyUp == aVisitor.mEvent->mMessage)) {
-            nsEventStatus status = nsEventStatus_eIgnore;
-
-            WidgetMouseEvent event(aVisitor.mEvent->mFlags.mIsTrusted,
-                                   eMouseClick, nullptr,
-                                   WidgetMouseEvent::eReal);
-            event.inputSource = nsIDOMMouseEvent::MOZ_SOURCE_KEYBOARD;
-            EventDispatcher::Dispatch(static_cast<nsIContent*>(this),
-                                      aVisitor.mPresContext, &event, nullptr,
-                                      &status);
+            DispatchSimulatedClick(this, aVisitor.mEvent->mFlags.mIsTrusted,
+                                   aVisitor.mPresContext);
             aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
           }
         }
         break;
 
       case eMouseDown:
         {
           WidgetMouseEvent* mouseEvent = aVisitor.mEvent->AsMouseEvent();
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@@ -3819,25 +3819,18 @@ HTMLInputElement::PostHandleEvent(EventC
                 MOZ_FALLTHROUGH;
               }
               case NS_FORM_INPUT_BUTTON:
               case NS_FORM_INPUT_RESET:
               case NS_FORM_INPUT_SUBMIT:
               case NS_FORM_INPUT_IMAGE: // Bug 34418
               case NS_FORM_INPUT_COLOR:
               {
-                WidgetMouseEvent event(aVisitor.mEvent->mFlags.mIsTrusted,
-                                       eMouseClick, nullptr,
-                                       WidgetMouseEvent::eReal);
-                event.inputSource = nsIDOMMouseEvent::MOZ_SOURCE_KEYBOARD;
-                nsEventStatus status = nsEventStatus_eIgnore;
-
-                EventDispatcher::Dispatch(static_cast<nsIContent*>(this),
-                                          aVisitor.mPresContext, &event,
-                                          nullptr, &status);
+                DispatchSimulatedClick(this, aVisitor.mEvent->mFlags.mIsTrusted,
+                                       aVisitor.mPresContext);
                 aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
               } // case
             } // switch
           }
           if (aVisitor.mEvent->mMessage == eKeyPress &&
               mType == NS_FORM_INPUT_RADIO && !keyEvent->IsAlt() &&
               !keyEvent->IsControl() && !keyEvent->IsMeta()) {
             bool isMovingBack = false;
@@ -3854,25 +3847,19 @@ HTMLInputElement::PostHandleEvent(EventC
                 nsAutoString name;
                 GetAttr(kNameSpaceID_None, nsGkAtoms::name, name);
                 RefPtr<HTMLInputElement> selectedRadioButton;
                 container->GetNextRadioButton(name, isMovingBack, this,
                                               getter_AddRefs(selectedRadioButton));
                 if (selectedRadioButton) {
                   rv = selectedRadioButton->Focus();
                   if (NS_SUCCEEDED(rv)) {
-                    nsEventStatus status = nsEventStatus_eIgnore;
-                    WidgetMouseEvent event(aVisitor.mEvent->mFlags.mIsTrusted,
-                                           eMouseClick, nullptr,
-                                           WidgetMouseEvent::eReal);
-                    event.inputSource = nsIDOMMouseEvent::MOZ_SOURCE_KEYBOARD;
-                    rv =
-                      EventDispatcher::Dispatch(ToSupports(selectedRadioButton),
-                                                aVisitor.mPresContext,
-                                                &event, nullptr, &status);
+                    rv = DispatchSimulatedClick(selectedRadioButton,
+                                                aVisitor.mEvent->mFlags.mIsTrusted,
+                                                aVisitor.mPresContext);
                     if (NS_SUCCEEDED(rv)) {
                       aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
                     }
                   }
                 }
               }
             }
           }
--- a/dom/html/HTMLSummaryElement.cpp
+++ b/dom/html/HTMLSummaryElement.cpp
@@ -5,16 +5,18 @@
 
 #include "mozilla/dom/HTMLSummaryElement.h"
 
 #include "mozilla/dom/HTMLDetailsElement.h"
 #include "mozilla/dom/HTMLElementBinding.h"
 #include "mozilla/dom/HTMLUnknownElement.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/TextEvents.h"
+#include "nsFocusManager.h"
 
 // Expand NS_IMPL_NS_NEW_HTML_ELEMENT(Summary) to add pref check.
 nsGenericHTMLElement*
 NS_NewHTMLSummaryElement(already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo,
                          mozilla::dom::FromParser aFromParser)
 {
   if (!mozilla::dom::HTMLDetailsElement::IsDetailsEnabled()) {
     return new mozilla::dom::HTMLUnknownElement(aNodeInfo);
@@ -39,42 +41,107 @@ HTMLSummaryElement::PostHandleEvent(Even
   if (!aVisitor.mPresContext) {
     return rv;
   }
 
   if (aVisitor.mEventStatus == nsEventStatus_eConsumeNoDefault) {
     return rv;
   }
 
-  auto toggleDetails = false;
-  auto* event = aVisitor.mEvent;
-
-  if (event->HasMouseEventMessage()) {
-    auto* mouseEvent = event->AsMouseEvent();
-    toggleDetails = mouseEvent->IsLeftClickEvent();
-  }
-
-  // Todo: Bug 634004: Implement toggle details by keyboard.
-
-  if (!toggleDetails || !IsMainSummary()) {
+  if (!IsMainSummary()) {
     return rv;
   }
 
-  auto* details = GetDetails();
-  MOZ_ASSERT(details, "Expected to find details since this is the main summary!");
+  WidgetEvent* const event = aVisitor.mEvent;
+
+  if (event->HasMouseEventMessage()) {
+    WidgetMouseEvent* mouseEvent = event->AsMouseEvent();
+
+    if (mouseEvent->IsLeftClickEvent()) {
+      RefPtr<HTMLDetailsElement> details = GetDetails();
+      MOZ_ASSERT(details,
+                 "Expected to find details since this is the main summary!");
+
+      // When dispatching a synthesized mouse click event to a details element
+      // with 'display: none', both Chrome and Safari do not toggle the 'open'
+      // attribute. We follow them by checking whether the details element has a
+      // frame or not.
+      if (details->GetPrimaryFrame(Flush_Frames)) {
+        details->ToggleOpen();
+        aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+        return NS_OK;
+      }
+    }
+  } // event->HasMouseEventMessage()
+
+  if (event->HasKeyEventMessage()) {
+    WidgetKeyboardEvent* keyboardEvent = event->AsKeyboardEvent();
+    bool dispatchClick = false;
+
+    switch (event->mMessage) {
+      case eKeyPress:
+        if (keyboardEvent->charCode == nsIDOMKeyEvent::DOM_VK_SPACE) {
+          // Consume 'space' key to prevent scrolling the page down.
+          aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+        }
 
-  // When dispatching a synthesized mouse click event to a details with
-  // 'display: none', both Chrome and Safari do not toggle the 'open' attribute.
-  // We follow them by checking whether details has a frame or not.
-  if (details->GetPrimaryFrame()) {
-    details->ToggleOpen();
-    aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+        dispatchClick = keyboardEvent->keyCode == nsIDOMKeyEvent::DOM_VK_RETURN;
+        break;
+
+      case eKeyUp:
+        dispatchClick = keyboardEvent->keyCode == nsIDOMKeyEvent::DOM_VK_SPACE;
+        break;
+
+      default:
+        break;
+    }
+
+    if (dispatchClick) {
+      rv = DispatchSimulatedClick(this, event->mFlags.mIsTrusted,
+                                  aVisitor.mPresContext);
+      if (NS_SUCCEEDED(rv)) {
+        aVisitor.mEventStatus = nsEventStatus_eConsumeNoDefault;
+      }
+    }
+  } // event->HasKeyEventMessage()
+
+  return rv;
+}
+
+bool
+HTMLSummaryElement::IsHTMLFocusable(bool aWithMouse, bool* aIsFocusable,
+                                    int32_t* aTabIndex)
+{
+  bool disallowOverridingFocusability =
+    nsGenericHTMLElement::IsHTMLFocusable(aWithMouse, aIsFocusable, aTabIndex);
+
+  if (disallowOverridingFocusability || !IsMainSummary()) {
+    return disallowOverridingFocusability;
   }
 
-  return rv;
+#ifdef XP_MACOSX
+  // The parent does not have strong opinion about the focusability of this main
+  // summary element, but we'd like to override it when mouse clicking on Mac OS
+  // like other form elements.
+  *aIsFocusable = !aWithMouse || nsFocusManager::sMouseFocusesFormControl;
+#else
+  // The main summary element is focusable on other platforms.
+  *aIsFocusable = true;
+#endif
+
+  // Give a chance to allow the subclass to override aIsFocusable.
+  return false;
+}
+
+int32_t
+HTMLSummaryElement::TabIndexDefault()
+{
+  // Make the main summary be able to navigate via tab, and be focusable.
+  // See nsGenericHTMLElement::IsHTMLFocusable().
+  return IsMainSummary() ? 0 : nsGenericHTMLElement::TabIndexDefault();
 }
 
 bool
 HTMLSummaryElement::IsMainSummary() const
 {
   HTMLDetailsElement* details = GetDetails();
   if (!details) {
     return false;
--- a/dom/html/HTMLSummaryElement.h
+++ b/dom/html/HTMLSummaryElement.h
@@ -28,16 +28,21 @@ public:
   }
 
   NS_IMPL_FROMCONTENT_HTML_WITH_TAG(HTMLSummaryElement, summary)
 
   nsresult Clone(NodeInfo* aNodeInfo, nsINode** aResult) const override;
 
   nsresult PostHandleEvent(EventChainPostVisitor& aVisitor) override;
 
+  bool IsHTMLFocusable(bool aWithMouse, bool* aIsFocusable,
+                       int32_t* aTabIndex) override;
+
+  int32_t TabIndexDefault() override;
+
   // Return true if this is the first summary element child of a details or the
   // default summary element generated by DetailsFrame.
   bool IsMainSummary() const;
 
   // Return the details element which contains this summary. Otherwise return
   // nullptr if there is no such details element.
   HTMLDetailsElement* GetDetails() const;
 
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -2800,30 +2800,35 @@ nsGenericHTMLElement::PerformAccesskey(b
 
     // Return true if the element became the current focus within its window.
     nsPIDOMWindowOuter* window = OwnerDoc()->GetWindow();
     focused = (window && window->GetFocusedNode());
   }
 
   if (aKeyCausesActivation) {
     // Click on it if the users prefs indicate to do so.
-    WidgetMouseEvent event(aIsTrustedEvent, eMouseClick, nullptr,
-                           WidgetMouseEvent::eReal);
-    event.inputSource = nsIDOMMouseEvent::MOZ_SOURCE_KEYBOARD;
-
     nsAutoPopupStatePusher popupStatePusher(aIsTrustedEvent ?
                                             openAllowed : openAbused);
-
-    EventDispatcher::Dispatch(static_cast<nsIContent*>(this),
-                              presContext, &event);
+    DispatchSimulatedClick(this, aIsTrustedEvent, presContext);
   }
 
   return focused;
 }
 
+nsresult
+nsGenericHTMLElement::DispatchSimulatedClick(nsGenericHTMLElement* aElement,
+                                             bool aIsTrusted,
+                                             nsPresContext* aPresContext)
+{
+  WidgetMouseEvent event(aIsTrusted, eMouseClick, nullptr,
+                         WidgetMouseEvent::eReal);
+  event.inputSource = nsIDOMMouseEvent::MOZ_SOURCE_KEYBOARD;
+  return EventDispatcher::Dispatch(ToSupports(aElement), aPresContext, &event);
+}
+
 const nsAttrName*
 nsGenericHTMLElement::InternalGetExistingAttrNameFromQName(const nsAString& aStr) const
 {
   if (IsInHTMLDocument()) {
     nsAutoString lower;
     nsContentUtils::ASCIIToLower(aStr, lower);
     return mAttrsAndChildren.GetExistingAttrNameFromQName(lower);
   }
--- a/dom/html/nsGenericHTMLElement.h
+++ b/dom/html/nsGenericHTMLElement.h
@@ -1023,16 +1023,23 @@ protected:
 
   virtual mozilla::EventListenerManager*
     GetEventListenerManagerForAttr(nsIAtom* aAttrName,
                                    bool* aDefer) override;
 
   virtual const nsAttrName* InternalGetExistingAttrNameFromQName(const nsAString& aStr) const override;
 
   /**
+   * Dispatch a simulated mouse click by keyboard to the given element.
+   */
+  nsresult DispatchSimulatedClick(nsGenericHTMLElement* aElement,
+                                  bool aIsTrusted,
+                                  nsPresContext* aPresContext);
+
+  /**
    * Create a URI for the given aURISpec string.
    * Returns INVALID_STATE_ERR and nulls *aURI if aURISpec is empty
    * and the document's URI matches the element's base URI.
    */
   nsresult NewURIFromString(const nsAString& aURISpec, nsIURI** aURI);
 
   void GetHTMLAttr(nsIAtom* aName, nsAString& aResult) const
   {
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/key-enter-open-second-summary.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html class="reftest-wait">
+  <script>
+  function runTest() {
+    // Dispatch 'return' key on second summary should not collapse details.
+    var summary = document.getElementById("summary");
+    summary.dispatchEvent(new KeyboardEvent("keypress", {"keyCode": 13}));
+    document.documentElement.removeAttribute("class");
+  }
+  </script>
+  <body onload="runTest();">
+    <details open>
+      <summary>Summary</summary>
+      <summary id="summary">Second Summary</summary>
+      <p>This is the details.</p>
+    </details>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/key-enter-prevent-default.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html class="reftest-wait">
+  <script>
+  function runTest() {
+    var summary = document.getElementById("summary");
+    summary.addEventListener('click', function(e) {
+      // Prevent the details from toggling by key events.
+      e.preventDefault();
+    });
+    // Dispatch 'return' key to the summary element.
+    summary.dispatchEvent(new KeyboardEvent("keypress", {"keyCode": 13}));
+    document.documentElement.removeAttribute("class");
+  }
+  </script>
+  <body onload="runTest();">
+    <details>
+      <summary id="summary">Summary</summary>
+      <p>This is the details.</p>
+    </details>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/key-enter-single-summary.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html class="reftest-wait">
+  <script>
+  function runTest() {
+    var summary = document.getElementById("summary");
+    // Dispatch 'return' key to the summary element.
+    summary.dispatchEvent(new KeyboardEvent("keypress", {"keyCode": 13}));
+    document.documentElement.removeAttribute("class");
+  }
+  </script>
+  <body onload="runTest();">
+    <details>
+      <summary id="summary">Summary</summary>
+      <p>This is the details.</p>
+    </details>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/details-summary/key-space-single-summary.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+
+<html class="reftest-wait">
+  <script>
+  function runTest() {
+    var summary = document.getElementById("summary");
+    // Dispatch 'space' keys to the summary element.
+    summary.dispatchEvent(new KeyboardEvent("keydown", {"keyCode": 32}));
+    summary.dispatchEvent(new KeyboardEvent("keyup", {"keyCode": 32}));
+    document.documentElement.removeAttribute("class");
+  }
+  </script>
+  <body onload="runTest();">
+    <details>
+      <summary id="summary">Summary</summary>
+      <p>This is the details.</p>
+    </details>
+  </body>
+</html>
--- a/layout/reftests/details-summary/reftest.list
+++ b/layout/reftests/details-summary/reftest.list
@@ -67,8 +67,14 @@ pref(dom.details_element.enabled,true) =
 pref(dom.details_element.enabled,true) == mouse-click-twice-overflow-auto-details.html overflow-auto-details.html
 pref(dom.details_element.enabled,true) == mouse-click-display-none-details.html single-summary.html
 
 # Dispatch mouse click to out-of-flow details or summary
 pref(dom.details_element.enabled,true) == mouse-click-fixed-summary.html open-fixed-summary.html
 pref(dom.details_element.enabled,true) == mouse-click-twice-fixed-summary.html fixed-summary.html
 pref(dom.details_element.enabled,true) == mouse-click-float-details.html open-float-details.html
 pref(dom.details_element.enabled,true) == mouse-click-twice-float-details.html float-details.html
+
+# Dispatch keyboard event to summary
+pref(dom.details_element.enabled,true) == key-enter-single-summary.html open-single-summary.html
+pref(dom.details_element.enabled,true) == key-enter-open-second-summary.html open-multiple-summary.html
+pref(dom.details_element.enabled,true) == key-enter-prevent-default.html single-summary.html
+pref(dom.details_element.enabled,true) == key-space-single-summary.html open-single-summary.html