Popup open and hide using Alt+KeyDown

Source/core/html/HTMLSelectElement.cpp

Index: Source/core/html/HTMLSelectElement.cpp
diff --git a/Source/core/html/HTMLSelectElement.cpp b/Source/core/html/HTMLSelectElement.cpp
index b0dc747ff14ab9cee6caa8a9264b2d0c2834ad1f..8b19b94765763dc83f28d86e71b85f1dbf830c46 100644
--- a/Source/core/html/HTMLSelectElement.cpp
+++ b/Source/core/html/HTMLSelectElement.cpp
@@ -1114,48 +1114,57 @@ void HTMLSelectElement::resetImpl()
     setNeedsValidityCheck();
 }
 
-#if !OS(WIN)
-bool HTMLSelectElement::platformHandleKeydownEvent(KeyboardEvent* event)
+bool HTMLSelectElement::handlePopupOpenKeyboardEvent(Event* event)
 {
-    if (!RenderTheme::theme().popsMenuByArrowKeys())
-        return false;
+    const KeyboardEvent* keyEvent = toKeyboardEvent(event);
+    const String& keyIdentifier = keyEvent->keyIdentifier();
+    RenderTheme& renderTheme = RenderTheme::theme();
+    int keyCode = toKeyboardEvent(event)->keyCode();
 
-    if (!isSpatialNavigationEnabled(document().frame())) {
-        if (event->keyIdentifier() == "Down" || event->keyIdentifier() == "Up") {
-            focus();
-            // Calling focus() may cause us to lose our renderer. Return true so
-            // that our caller doesn't process the event further, but don't set
-            // the event as handled.
-            if (!renderer() || !renderer()->isMenuList() || isDisabledFormControl())
-                return true;
+    // downUpKey and spaceKey works for mac.
+    bool downUpKey = (event->type() == EventTypeNames::keydown) && !isSpatialNavigationEnabled(document().frame()) && RenderTheme::theme().popsMenuByArrowKeys() && (keyIdentifier == "Down" || keyIdentifier == "Up");
+    bool spaceKey = (event->type() == EventTypeNames::keypress) &&  renderTheme.popsMenuByArrowKeys() && keyCode == ' ';

[keishi, 2014/07/04 01:33:50]

Using RenderTheme.popsMenuByArrowKeys here seems wrong. Maybe you can split
popsMenuBySpaceOrReturn to popsMenuBySpace and popsMenuByReturn

[Habib Virji, 2014/07/08 12:21:32]

Done.

+    // spaceOrEnterKey, altDownUpKey and f4Key all work for linux and windows but not for mac.
+    bool spaceOrEnterKey = (event->type() == EventTypeNames::keypress) && renderTheme.popsMenuBySpaceOrReturn() && (keyCode == ' ' || keyCode == '\r');
+    bool altDownUpKey = (event->type() == EventTypeNames::keydown) && renderTheme.shouldOpenDropDownWithAltDownUpOrF4Key() && (keyIdentifier == "Down" || keyIdentifier == "Up") && (keyEvent->altKey() || keyEvent->altGraphKey());
+    bool f4Key = (event->type() == EventTypeNames::keydown) && renderTheme.shouldOpenDropDownWithAltDownUpOrF4Key() && (!keyEvent->altKey() && !keyEvent->ctrlKey() && keyIdentifier == "F4");
 
-            // Save the selection so it can be compared to the new selection
-            // when dispatching change events during selectOption, which
-            // gets called from RenderMenuList::valueChanged, which gets called
-            // after the user makes a selection from the menu.
-            saveLastSelection();
-            if (RenderMenuList* menuList = toRenderMenuList(renderer()))
-                menuList->showPopup();
-            event->setDefaultHandled();
-        }
+    if (downUpKey || spaceKey || spaceOrEnterKey || altDownUpKey || f4Key) {

[keishi, 2014/07/04 01:33:50]

nit: I think this is kind of confusing because spaceKey may be false even when
space is pressed.
Maybe we could have a single shouldOpenPopup boolean and do |=.

bool shouldOpenPopup = false;
if (event->type() == EventTypeNames::keypress)
  shouldOpenPopup |= renderTheme.popsMenuByArrowKeys() && keyCode == ' ';
  ...
} else if (event->type() == EventTypeNames::keydown)
  ...
}
if (shouldOpenPopup) {
  // open popup
} else {
  // Maybe you could event handle form submission and spatial navigation
selecting in here too.
}

[Habib Virji, 2014/07/08 12:21:32]

Done.

+        focus();
+        // Calling focus() may cause us to lose our renderer. Return true so
+        // that our caller doesn't process the event further, but don't set
+        // the event as handled.
+        if (!renderer() || !renderer()->isMenuList() || isDisabledFormControl())
+            return true;
+        // Save the selection so it can be compared to the new selection
+        // when dispatching change events during selectOption, which
+        // gets called from RenderMenuList::valueChanged, which gets called
+        // after the user makes a selection from the menu.
+        saveLastSelection();
+        if (RenderMenuList* menuList = toRenderMenuList(renderer()))
+            menuList->showPopup();
+        int index = selectedIndex();
+        ASSERT(index >= 0);
+        ASSERT_WITH_SECURITY_IMPLICATION(index < static_cast<int>(listItems().size()));
+        setSelectedIndex(index);
+        event->setDefaultHandled();
         return true;
     }
 
     return false;
 }
-#endif
 
 void HTMLSelectElement::menuListDefaultEventHandler(Event* event)
 {
     RenderTheme& renderTheme = RenderTheme::theme();
 
+    if (renderer() && event->isKeyboardEvent() && handlePopupOpenKeyboardEvent(event))
+        return;
+
     if (event->type() == EventTypeNames::keydown) {
         if (!renderer() || !event->isKeyboardEvent())
             return;
 
-        if (platformHandleKeydownEvent(toKeyboardEvent(event)))
-            return;
-
         // When using spatial navigation, we want to be able to navigate away
         // from the select element when the user hits any of the arrow keys,
         // instead of changing the selection.
@@ -1198,7 +1207,6 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* event)
             return;
 
         int keyCode = toKeyboardEvent(event)->keyCode();
-        bool handled = false;
 
         if (keyCode == ' ' && isSpatialNavigationEnabled(document().frame())) {
             // Use space to toggle arrow key handling for selection change or spatial navigation.
@@ -1207,51 +1215,12 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* event)
             return;
         }
 
-        if (renderTheme.popsMenuBySpaceOrReturn()) {
-            if (keyCode == ' ' || keyCode == '\r') {
-                focus();
-
-                // Calling focus() may remove the renderer or change the
-                // renderer type.
-                if (!renderer() || !renderer()->isMenuList() || isDisabledFormControl())
-                    return;
-
-                // Save the selection so it can be compared to the new selection
-                // when dispatching change events during selectOption, which
-                // gets called from RenderMenuList::valueChanged, which gets called
-                // after the user makes a selection from the menu.
-                saveLastSelection();
-                if (RenderMenuList* menuList = toRenderMenuList(renderer()))
-                    menuList->showPopup();
-                handled = true;
-            }
-        } else if (renderTheme.popsMenuByArrowKeys()) {
-            if (keyCode == ' ') {
-                focus();
-
-                // Calling focus() may remove the renderer or change the
-                // renderer type.
-                if (!renderer() || !renderer()->isMenuList() || isDisabledFormControl())
-                    return;
-
-                // Save the selection so it can be compared to the new selection
-                // when dispatching change events during selectOption, which
-                // gets called from RenderMenuList::valueChanged, which gets called
-                // after the user makes a selection from the menu.
-                saveLastSelection();
-                if (RenderMenuList* menuList = toRenderMenuList(renderer()))
-                    menuList->showPopup();
-                handled = true;
-            } else if (keyCode == '\r') {
-                if (form())
-                    form()->submitImplicitly(event, false);
-                dispatchInputAndChangeEventForMenuList();
-                handled = true;
-            }
-        }
-
-        if (handled)
+        if (renderTheme.popsMenuByArrowKeys() && keyCode == '\r') {
+            if (form())
+                form()->submitImplicitly(event, false);
+            dispatchInputAndChangeEventForMenuList();
             event->setDefaultHandled();
+        }
     }
 
     if (event->type() == EventTypeNames::mousedown && event->isMouseEvent() && toMouseEvent(event)->button() == LeftButton) {