Move arrow scrolling into KeyboardEventManager.

third_party/WebKit/Source/core/input/KeyboardEventManager.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "KeyboardEventManager.h"
 
 #include "core/dom/Element.h"
 #include "core/editing/Editor.h"
 #include "core/events/KeyboardEvent.h"
 #include "core/html/HTMLDialogElement.h"
+#include "core/input/EventHandler.h"
 #include "core/input/EventHandlingUtil.h"
 #include "core/input/ScrollManager.h"
 #include "core/layout/LayoutObject.h"
 #include "core/layout/LayoutTextControlSingleLine.h"
 #include "core/loader/FrameLoaderClient.h"
 #include "core/page/ChromeClient.h"
 #include "core/page/FocusController.h"
 #include "core/page/Page.h"
 #include "core/page/SpatialNavigation.h"
+#include "platform/KeyboardCodes.h"
 #include "platform/UserGestureIndicator.h"
 #include "platform/WindowsKeyboardCodes.h"
 #include "public/platform/WebInputEvent.h"
 
 #if OS(WIN)
 #include <windows.h>
 #elif OS(MACOSX)
 #import <Carbon/Carbon.h>
 #endif
 
@@ skipping 13 matching lines @@
     retVal = WebFocusTypeDown;
   else if (key == "ArrowUp")
     retVal = WebFocusTypeUp;
   else if (key == "ArrowLeft")
     retVal = WebFocusTypeLeft;
   else if (key == "ArrowRight")
     retVal = WebFocusTypeRight;
   return retVal;
 }
 
+bool mapKeyCodeForScroll(int keyCode,
+                         PlatformEvent::Modifiers modifiers,
+                         ScrollDirection* scrollDirection,
+                         ScrollGranularity* scrollGranularity) {
+  if (modifiers & PlatformEvent::ShiftKey || modifiers & PlatformEvent::MetaKey)
+    return false;
+
+  if (modifiers & PlatformEvent::AltKey) {
+    // Alt-Up/Down should behave like PageUp/Down on Mac.  (Note that Alt-keys
+    // on other platforms are suppressed due to isSystemKey being set.)

[skobes, 2016/10/26 17:57:44]

Why do Alt-keys not set isSystemKey on Mac?

I wonder if it's safer to leave the #if guard on this, since it is Mac specific.

[aelias_OOO_until_Jul13, 2016/10/26 19:27:39]

On Mac, the system key is the meta key instead:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/web_...

I have a strong bias against #if guards because they make unit testing much more
painful (issues only appear on platform trybots, not locally).  AFAIK this is
safe and well-aligned with the intent of the platform-specific suppression.  If
there's a regression reported out of this, I'll be happy to fix.

+    if (keyCode == VKEY_UP)
+      keyCode = VKEY_PRIOR;
+    else if (keyCode == VKEY_DOWN)
+      keyCode = VKEY_NEXT;
+    else
+      return false;
+  }
+
+  if (modifiers & PlatformEvent::CtrlKey) {
+    // Match FF behavior in the sense that Ctrl+home/end are the only Ctrl
+    // key combinations which affect scrolling.
+    if (keyCode != VKEY_HOME && keyCode != VKEY_END)
+      return false;
+  }
+
+  switch (keyCode) {
+    case VKEY_LEFT:
+      *scrollDirection = ScrollLeftIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_RIGHT:
+      *scrollDirection = ScrollRightIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_UP:
+      *scrollDirection = ScrollUpIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_DOWN:
+      *scrollDirection = ScrollDownIgnoringWritingMode;
+      *scrollGranularity = ScrollByLine;
+      break;
+    case VKEY_HOME:
+      *scrollDirection = ScrollUpIgnoringWritingMode;
+      *scrollGranularity = ScrollByDocument;
+      break;
+    case VKEY_END:
+      *scrollDirection = ScrollDownIgnoringWritingMode;
+      *scrollGranularity = ScrollByDocument;
+      break;
+    case VKEY_PRIOR:  // page up
+      *scrollDirection = ScrollUpIgnoringWritingMode;
+      *scrollGranularity = ScrollByPage;
+      break;
+    case VKEY_NEXT:  // page down
+      *scrollDirection = ScrollDownIgnoringWritingMode;
+      *scrollGranularity = ScrollByPage;
+      break;
+    default:
+      return false;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 KeyboardEventManager::KeyboardEventManager(LocalFrame* frame,
                                            ScrollManager* scrollManager)
     : m_frame(frame), m_scrollManager(scrollManager) {}
 
 DEFINE_TRACE(KeyboardEventManager) {
   visitor->trace(m_frame);
   visitor->trace(m_scrollManager);
 }
@@ skipping 140 matching lines @@
     // TODO(dtapuska): Replace this with isComposing support. crbug.com/625686
     if (event->keyCode() == kVKeyProcessKey)
       return;
     if (event->key() == "Tab") {
       defaultTabEventHandler(event);
     } else if (event->key() == "Backspace") {
       defaultBackspaceEventHandler(event);
     } else if (event->key() == "Escape") {
       defaultEscapeEventHandler(event);
     } else {
-      WebFocusType type = focusDirectionForKey(event->key());
-      if (type != WebFocusTypeNone)
-        defaultArrowEventHandler(type, event);
+      defaultArrowEventHandler(event, possibleFocusedNode);
     }
   }
   if (event->type() == EventTypeNames::keypress) {
     m_frame->editor().handleKeyboardEvent(event);
     if (event->defaultHandled())
       return;
     if (event->charCode() == ' ')
       defaultSpaceEventHandler(event, possibleFocusedNode);
   }
 }
@@ skipping 30 matching lines @@
   UseCounter::count(m_frame->document(), UseCounter::BackspaceNavigatedBack);
   if (m_frame->page()->chromeClient().hadFormInteraction())
     UseCounter::count(m_frame->document(),
                       UseCounter::BackspaceNavigatedBackAfterFormInteraction);
   bool handledEvent = m_frame->loader().client()->navigateBackForward(
       event->shiftKey() ? 1 : -1);
   if (handledEvent)
     event->setDefaultHandled();
 }
 
-void KeyboardEventManager::defaultArrowEventHandler(WebFocusType focusType,
-                                                    KeyboardEvent* event) {
+void KeyboardEventManager::defaultArrowEventHandler(KeyboardEvent* event,
+                                                    Node* possibleFocusedNode) {
   DCHECK_EQ(event->type(), EventTypeNames::keydown);
 
-  if (event->ctrlKey() || event->metaKey() || event->shiftKey())
-    return;
-
   Page* page = m_frame->page();
   if (!page)
     return;
 
-  if (!isSpatialNavigationEnabled(m_frame))
+  WebFocusType type = focusDirectionForKey(event->key());
+  if (event->ctrlKey() || event->metaKey() || event->shiftKey())

[skobes, 2016/10/26 17:57:44]

Maybe this check should be in focusDirectionForKey (and pass it the event).

[aelias_OOO_until_Jul13, 2016/10/26 19:27:39]

Done.

+    type = WebFocusTypeNone;
+  if (type != WebFocusTypeNone && isSpatialNavigationEnabled(m_frame) &&
+      !m_frame->document()->inDesignMode()) {
+    if (page->focusController().advanceFocus(type)) {
+      event->setDefaultHandled();
+      return;
+    }
+  }
+
+  if (event->keyEvent()->isSystemKey)
     return;
 
-  // Arrows and other possible directional navigation keys can be used in design
-  // mode editing.
-  if (m_frame->document()->inDesignMode())
+  ScrollDirection scrollDirection;
+  ScrollGranularity scrollGranularity;
+  if (!mapKeyCodeForScroll(event->keyCode(), event->modifiers(),
+                           &scrollDirection, &scrollGranularity))
     return;
 
-  if (page->focusController().advanceFocus(focusType))
+  if (m_scrollManager->bubblingScroll(scrollDirection, scrollGranularity,
+                                      nullptr, possibleFocusedNode)) {
     event->setDefaultHandled();
+    return;
+  }
 }
 
 void KeyboardEventManager::defaultTabEventHandler(KeyboardEvent* event) {
   DCHECK_EQ(event->type(), EventTypeNames::keydown);
 
   // We should only advance focus on tabs if no special modifier keys are held
   // down.
   if (event->ctrlKey() || event->metaKey())
     return;
 
@@ skipping 75 matching lines @@
   if (currentModifiers & ::cmdKey)
     modifiers |= WebInputEvent::MetaKey;
 #else
   // TODO(crbug.com/538289): Implement on other platforms.
   return static_cast<WebInputEvent::Modifiers>(0);
 #endif
   return static_cast<WebInputEvent::Modifiers>(modifiers);
 }
 
 }  // namespace blink