Bug 1327129 - Make key events act on the first non-disabled <option> (if any) when no <option> is selected (i.e. selectedIndex=-1). r=smaug
authorMats Palmgren <mats@mozilla.com>
Sun, 08 Jan 2017 21:26:59 +0100
changeset 375767 fb6c4863ab27d5957058666fb9ffac251148bf92
parent 375766 f037753a1db2db25ebf344fbd8cfedfb28e10958
child 375768 5aa1f8402f8cdf9b86d09e0972fe147df325092f
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1327129
milestone53.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 1327129 - Make key events act on the first non-disabled <option> (if any) when no <option> is selected (i.e. selectedIndex=-1). r=smaug
layout/forms/nsListControlFrame.cpp
layout/forms/nsListControlFrame.h
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -1117,31 +1117,40 @@ nsListControlFrame::GetCurrentOption()
   // the selected index if this is kNothingSelected.
   int32_t focusedIndex = (mEndSelectionIndex == kNothingSelected) ?
     GetSelectedIndex() : mEndSelectionIndex;
 
   if (focusedIndex != kNothingSelected) {
     return GetOption(AssertedCast<uint32_t>(focusedIndex));
   }
 
-  // There is no selected item. Return the first non-disabled item.
+  // There is no selected option. Return the first non-disabled option, if any.
+  return GetNonDisabledOptionFrom(0);
+}
+
+HTMLOptionElement*
+nsListControlFrame::GetNonDisabledOptionFrom(int32_t aFromIndex,
+                                             int32_t* aFoundIndex)
+{
   RefPtr<dom::HTMLSelectElement> selectElement =
     dom::HTMLSelectElement::FromContent(mContent);
 
-  for (uint32_t i = 0, length = selectElement->Length(); i < length; ++i) {
-    dom::HTMLOptionElement* node = selectElement->Item(i);
+  const uint32_t length = selectElement->Length();
+  for (uint32_t i = std::max(aFromIndex, 0); i < length; ++i) {
+    HTMLOptionElement* node = selectElement->Item(i);
     if (!node) {
-      return nullptr;
+      break;
     }
-
     if (!selectElement->IsOptionDisabled(node)) {
+      if (aFoundIndex) {
+        *aFoundIndex = i;
+      }
       return node;
     }
   }
-
   return nullptr;
 }
 
 bool
 nsListControlFrame::IsInDropDownMode() const
 {
   return (mComboboxFrame != nullptr);
 }
@@ -2474,17 +2483,27 @@ nsListControlFrame::KeyPress(nsIDOMEvent
 
 void
 nsListControlFrame::PostHandleKeyEvent(int32_t aNewIndex,
                                        uint32_t aCharCode,
                                        bool aIsShift,
                                        bool aIsControlOrMeta)
 {
   if (aNewIndex == kNothingSelected) {
-    return;
+    int32_t focusedIndex = mEndSelectionIndex == kNothingSelected ?
+      GetSelectedIndex() : mEndSelectionIndex;
+    if (focusedIndex != kNothingSelected) {
+      return;
+    }
+    // No options are selected.  In this case the focus ring is on the first
+    // non-disabled option (if any), so we should behave as if that's the option
+    // the user acted on.
+    if (!GetNonDisabledOptionFrom(0, &aNewIndex)) {
+      return;
+    }
   }
 
   // If you hold control, but not shift, no key will actually do anything
   // except space.
   nsWeakFrame weakFrame(this);
   bool wasChanged = false;
   if (aIsControlOrMeta && !aIsShift && aCharCode != ' ') {
     mStartSelectionIndex = aNewIndex;
--- a/layout/forms/nsListControlFrame.h
+++ b/layout/forms/nsListControlFrame.h
@@ -44,16 +44,18 @@ class HTMLOptionsCollection;
  */
 
 class nsListControlFrame final : public nsHTMLScrollFrame,
                                  public nsIFormControlFrame,
                                  public nsIListControlFrame,
                                  public nsISelectControlFrame
 {
 public:
+  typedef mozilla::dom::HTMLOptionElement HTMLOptionElement;
+
   friend nsContainerFrame* NS_NewListControlFrame(nsIPresShell* aPresShell,
                                                   nsStyleContext* aContext);
 
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS
 
     // nsIFrame
   virtual nsresult HandleEvent(nsPresContext* aPresContext,
@@ -113,17 +115,17 @@ public:
     // for accessibility purposes
 #ifdef ACCESSIBILITY
   virtual mozilla::a11y::AccType AccessibleType() override;
 #endif
 
     // nsIListControlFrame
   virtual void SetComboboxFrame(nsIFrame* aComboboxFrame) override;
   virtual int32_t GetSelectedIndex() override;
-  virtual mozilla::dom::HTMLOptionElement* GetCurrentOption() override;
+  virtual HTMLOptionElement* GetCurrentOption() override;
 
   /**
    * Gets the text of the currently selected item.
    * If the there are zero items then an empty string is returned
    * If there is nothing selected, then the 0th item's text is returned.
    */
   virtual void GetOptionText(uint32_t aIndex, nsAString& aStr) override;
 
@@ -175,17 +177,17 @@ public:
 
   /**
    * Returns the options collection for mContent, if any.
    */
   mozilla::dom::HTMLOptionsCollection* GetOptions() const;
   /**
    * Returns the HTMLOptionElement for a given index in mContent's collection.
    */
-  mozilla::dom::HTMLOptionElement* GetOption(uint32_t aIndex) const;
+  HTMLOptionElement* GetOption(uint32_t aIndex) const;
 
   static void ComboboxFocusSet();
 
   // Helper
   bool IsFocused() { return this == mFocused; }
 
   /**
    * Function to paint the focus rect when our nsSelectsAreaFrame is painting.
@@ -253,16 +255,23 @@ public:
    * fire a native focus event for accessibility
    * (Some 3rd party products need to track our focus)
    */
   void FireMenuItemActiveEvent(); // Inform assistive tech what got focused
 #endif
 
 protected:
   /**
+   * Return the first non-disabled option starting at aFromIndex (inclusive).
+   * @param aFoundIndex if non-null, set to the index of the returned option 
+   */
+  HTMLOptionElement* GetNonDisabledOptionFrom(int32_t aFromIndex,
+                                              int32_t* aFoundIndex = nullptr);
+
+  /**
    * Updates the selected text in a combobox and then calls FireOnChange().
    * @note This method might destroy the frame, pres shell and other objects.
    * Returns false if calling it destroyed |this|.
    */
   bool       UpdateSelection();
 
   /**
    * Returns whether mContent supports multiple selection.
@@ -277,17 +286,17 @@ protected:
    * @note This method might destroy the frame, pres shell and other objects.
    */
   void       DropDownToggleKey(nsIDOMEvent* aKeyEvent);
 
   nsresult   IsOptionDisabled(int32_t anIndex, bool &aIsDisabled);
   /**
    * @note This method might destroy the frame, pres shell and other objects.
    */
-  void ScrollToFrame(mozilla::dom::HTMLOptionElement& aOptElement);
+  void ScrollToFrame(HTMLOptionElement& aOptElement);
   /**
    * @note This method might destroy the frame, pres shell and other objects.
    */
   void ScrollToIndex(int32_t anIndex);
 
   /**
    * When the user clicks on the comboboxframe to show the dropdown
    * listbox, they then have to move the mouse into the list. We don't