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..3ad316b403fffee81bb44f465420eba16be65f56 100644
--- a/Source/core/html/HTMLSelectElement.cpp
+++ b/Source/core/html/HTMLSelectElement.cpp
@@ -1114,48 +1114,90 @@ void HTMLSelectElement::resetImpl()
     setNeedsValidityCheck();
 }
 
-#if !OS(WIN)
-bool HTMLSelectElement::platformHandleKeydownEvent(KeyboardEvent* event)
+bool HTMLSelectElement::handlePopupOpenKeyboardEvent(Event* event)

[keishi, 2014/07/09 03:38:21]

HTMLSelectElement::handlePopupOpenKeyboardEvent has stuff not related to opening
the popup and as you mention in the comments the early return stuff is tricky.

Maybe the code will be easier to follow if most of the structure was kept in
HTMLSelectElement::menuListDefaultEventHandler.

For example:

// Return true if 
bool HTMLSelectElement::handlePopupOpenKeyboardEvent(KeyboardEvent* event)
{
    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 false;
}

bool HTMLSelectElement::shouldOpenPopupForKeyDownEvent(KeyboardEvent* event)
{
    const String& keyIdentifier = event->keyIdentifier();
    RenderTheme& renderTheme = RenderTheme::theme();
    int keyCode = event->keyCode();
    if (isSpatialNavigationEnabled(document().frame()))
        return false;
    return (renderTheme.popsMenuByArrowKeys() &&  (keyIdentifier == "Down" ||
keyIdentifier == "Up"))
        || (renderTheme.popsMenuByAltDownUpOrF4Key() && (keyIdentifier == "Down"
|| keyIdentifier == "Up") && (event->altKey() || event->altGraphKey()))
        || (renderTheme.popsMenuByAltDownUpOrF4Key() && (!event->altKey() &&
!event->ctrlKey() && keyIdentifier == "F4"));
}

bool HTMLSelectElement::shouldOpenPopupForKeyPressEvent(KeyboardEvent* event)
{
    const String& keyIdentifier = event->keyIdentifier();
    RenderTheme& renderTheme = RenderTheme::theme();
    int keyCode = event->keyCode();
    return ((renderTheme.popsMenuBySpaceKey() && keyCode == ' ')
        || (!renderTheme.popsMenuByArrowKeys() &&
renderTheme.popsMenuByReturnKey() && keyCode == '\r'));
}

void HTMLSelectElement::menuListDefaultEventHandler(Event* event)
{
    ...
    if (event->type() == EventTypeNames::keydown) {
        if (!renderer() || !event->isKeyboardEvent())
            return;

        KeyboardEvent* keyEvent = toKeyboardEvent(event);
        if (shouldOpenPopupForKeyDownEvent(keyEvent)) {
            handlePopupOpenKeyboardEvent(keyEvent);
            return;
        }
        ...
    }
    if (event->type() == EventTypeNames::keypress) {
        if (!renderer() || !event->isKeyboardEvent())
            return;

        int keyCode = toKeyboardEvent(event)->keyCode();
        if (keyCode == ' ' && isSpatialNavigationEnabled(document().frame())) {
            // Use space to toggle arrow key handling for selection change or
spatial navigation.
            m_activeSelectionState = !m_activeSelectionState;
            event->setDefaultHandled();
            return;
        }
        
        KeyboardEvent* keyEvent = toKeyboardEvent(event);
        if (shouldOpenPopupForKeyPressEvent(keyEvent)) {
            handlePopupOpenKeyboardEvent(keyEvent);
            return;
        }
    }
    ...
}

[Habib Virji, 2014/07/09 09:49:39]

Done.

 {
-    if (!RenderTheme::theme().popsMenuByArrowKeys())
-        return false;
+    bool shouldOpenPopup = false;
+    const KeyboardEvent* keyEvent = toKeyboardEvent(event);
+    const String& keyIdentifier = keyEvent->keyIdentifier();
+    RenderTheme& renderTheme = RenderTheme::theme();
+    int keyCode = toKeyboardEvent(event)->keyCode();

[keishi, 2014/07/09 03:38:21]

keyEvent

[Habib Virji, 2014/07/09 09:49:39]

Done.

 
-    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;
+    if (event->type() == EventTypeNames::keypress) {
+        if (keyCode == ' ' && isSpatialNavigationEnabled(document().frame())) {
+            // Use space to toggle arrow key handling for selection change or spatial navigation.
+            m_activeSelectionState = !m_activeSelectionState;
+            event->setDefaultHandled();
+            return true;
+        }
+        // space works for all platforms
+        // enter works for only linux and windows.
+        shouldOpenPopup |= ((renderTheme.popsMenuBySpaceKey() && keyCode == ' ')
+            || (!renderTheme.popsMenuByArrowKeys() && renderTheme.popsMenuByReturnKey() && keyCode == '\r')); // popsMenuByArrowKeys() is true only for mac

[keishi, 2014/07/09 03:38:21]

popsMenuByReturnKey() should be false on mac so you don't need to check
popsMenuByArrowKeys

[Habib Virji, 2014/07/09 09:49:39]

Done.

+    }
 
-            // 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();
+    if (event->type() == EventTypeNames::keydown) {
+        // 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.
+        if (isSpatialNavigationEnabled(document().frame())) {
+            return !m_activeSelectionState ? true : false; // false since we want it to continue
+        }
+        // down/Up works for mac.
+        // down/up+alt & f4 works for linux and windows

[keishi, 2014/07/09 03:38:21]

I don't think we need this comment because we aren't doing anything special.
This is written in RenderTheme.

[Habib Virji, 2014/07/09 09:49:39]

Done.

+        shouldOpenPopup |= (renderTheme.popsMenuByArrowKeys() &&  (keyIdentifier == "Down" || keyIdentifier == "Up"))
+            || (renderTheme.popsMenuByAltDownUpOrF4Key() && (keyIdentifier == "Down" || keyIdentifier == "Up") && (keyEvent->altKey() || keyEvent->altGraphKey()))
+            || (renderTheme.popsMenuByAltDownUpOrF4Key() && (!keyEvent->altKey() && !keyEvent->ctrlKey() && keyIdentifier == "F4"));
+    }
+
+    if (shouldOpenPopup) {
+        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;
+    }
+
+    // This is mac specific handling
+    if (renderTheme.popsMenuByArrowKeys() && !isSpatialNavigationEnabled(document().frame())) {
+        // Use key press event here since sending simulated mouse events
+        // on key down blocks the proper sending of the out key press event.
+        if (event->type() == EventTypeNames::keypress && renderTheme.popsMenuByReturnKey() && keyCode == '\r') {
+            if (form())
+                form()->submitImplicitly(event, false);
+            dispatchInputAndChangeEventForMenuList();
             event->setDefaultHandled();
         }
+        // In mac scenario, we do not want to handle any other key handling.
+        // return true here for early return
         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.
@@ -1191,69 +1233,6 @@ void HTMLSelectElement::menuListDefaultEventHandler(Event* event)
             event->setDefaultHandled();
     }
 
-    // Use key press event here since sending simulated mouse events
-    // on key down blocks the proper sending of the key press event.
-    if (event->type() == EventTypeNames::keypress) {
-        if (!renderer() || !event->isKeyboardEvent())
-            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.
-            m_activeSelectionState = !m_activeSelectionState;
-            event->setDefaultHandled();
-            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)
-            event->setDefaultHandled();
-    }
-
     if (event->type() == EventTypeNames::mousedown && event->isMouseEvent() && toMouseEvent(event)->button() == LeftButton) {
         focus();
         if (renderer() && renderer()->isMenuList() && !isDisabledFormControl()) {