Changeset 83522 in webkit

Timestamp:

Apr 11, 2011 3:41:52 PM (13 years ago)

Author:

ap@apple.com

Message:

Reviewed by Maciej Stachowiak.

WebKit2: Cannot use Ctrl-Delete as a custom keyboard shortcut
​https://bugs.webkit.org/show_bug.cgi?id=58265
<rdar://problem/9221468>

• WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::keyEvent): Added a comment explaining that doing work after DOM event dispatch isn't great.
• WebProcess/WebPage/mac/WebPageMac.mm: (WebKit::WebPage::performNonEditingBehaviorForSelector): Added a comment explaining how this might be moved down to WebCore. (WebKit::WebPage::performDefaultBehaviorForKeyEvent): This is now empty, since both Space and Backspace behaviors are implemented in WebCore.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/EventHandler.cpp (modified) (4 diffs)
• WebCore/page/EventHandler.h (modified) (1 diff)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/win/ChangeLog (modified) (1 diff)
• WebKit/win/WebView.cpp (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• WebKit2/WebProcess/WebPage/mac/WebPageMac.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-04-11  Alexey Proskuryakov  <ap@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        WebKit2: Cannot use Ctrl-Delete as a custom keyboard shortcut
+        https://bugs.webkit.org/show_bug.cgi?id=58265
+        <rdar://problem/9221468>
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::defaultKeyboardEventHandler): Handle Backspace.
+        (WebCore::EventHandler::defaultSpaceEventHandler): Enabled this on Mac, and added checks for
+        other modifiers - no one wants to scroll on Ctrl+Alt+Space and such.
+        (WebCore::EventHandler::defaultBackspaceEventHandler): Backspace goes back, Shift-backspace
+        goes forward. All browsers do it.
+        (WebCore::EventHandler::defaultArrowEventHandler): Assert that this function was called
+        for a correct event.
+        (WebCore::EventHandler::defaultTabEventHandler): Ditto.
+
+        * page/EventHandler.h: Added defaultBackspaceEventHandler.
+
 2011-04-11  Matthew Delaney  <mdelaney@apple.com>
 

trunk/Source/WebCore/page/EventHandler.cpp

@@ -2545 +2545 @@
         if (event->keyIdentifier() == "U+0009")
             defaultTabEventHandler(event);
+        else if (event->keyIdentifier() == "U+0008")
+            defaultBackspaceEventHandler(event);
         else {
             FocusDirection direction = focusDirectionForKey(event->keyIdentifier());
@@ -2846 +2848 @@
 }
 
-#if PLATFORM(QT) || PLATFORM(MAC)
-
-// These two platforms handle the space event in the platform-specific WebKit code.
-// Eventually it would be good to eliminate that and use the code here instead, but
-// the Qt version is inside an ifdef and the Mac version has some extra behavior
-// so we can't unify everything yet.
+#if PLATFORM(QT)
+// Qt handles the space event in platform-specific WebKit code.
+// Eventually it would be good to eliminate that and use the code here instead.
 void EventHandler::defaultSpaceEventHandler(KeyboardEvent*)
 {
 }
-
 #else
 
 void EventHandler::defaultSpaceEventHandler(KeyboardEvent* event)
 {
+    ASSERT(event->type() == eventNames().keypressEvent);
+
+    if (event->ctrlKey() || event->metaKey() || event->altKey() || event->altGraphKey())
+        return;
+
     ScrollLogicalDirection direction = event->shiftKey() ? ScrollBlockDirectionBackward : ScrollBlockDirectionForward;
     if (logicalScrollOverflow(direction, ScrollByPage)) {
@@ -2876 +2879 @@
 #endif
 
+void EventHandler::defaultBackspaceEventHandler(KeyboardEvent* event)
+{
+    ASSERT(event->type() == eventNames().keydownEvent);
+
+    if (event->ctrlKey() || event->metaKey() || event->altKey() || event->altGraphKey())
+        return;
+
+    Page* page = m_frame->page();
+    if (!page)
+        return;
+
+    if (event->shiftKey())
+        page->goForward();
+    else
+        page->goBack();
+
+    event->setDefaultHandled();
+}
+
+
 void EventHandler::defaultArrowEventHandler(FocusDirection focusDirection, KeyboardEvent* event)
 {
+    ASSERT(event->type() == eventNames().keydownEvent);
+
     if (event->ctrlKey() || event->metaKey() || event->altGraphKey() || event->shiftKey())
         return;
@@ -2899 +2924 @@
 void EventHandler::defaultTabEventHandler(KeyboardEvent* event)
 {
+    ASSERT(event->type() == eventNames().keydownEvent);
+
     // We should only advance focus on tabs if no special modifier keys are held down.
     if (event->ctrlKey() || event->metaKey() || event->altGraphKey())

trunk/Source/WebCore/page/EventHandler.h

@@ -346 +346 @@
 
     void defaultSpaceEventHandler(KeyboardEvent*);
+    void defaultBackspaceEventHandler(KeyboardEvent*);
     void defaultTabEventHandler(KeyboardEvent*);
     void defaultArrowEventHandler(FocusDirection, KeyboardEvent*);

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2011-04-11  Alexey Proskuryakov  <ap@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        WebKit2: Safari doesn't respect cmd-arrows (and variations) as custom keyboard shortcuts
+        https://bugs.webkit.org/show_bug.cgi?id=58175
+        <rdar://problem/9060555>
+
+        * WebView/WebFrameView.mm: (-[WebFrameView keyDown:]): Added a comment explaining how this
+        should be changed to match WebKit2.
+
 2011-04-06  Timothy Hatcher  <timothy@apple.com>
 

trunk/Source/WebKit/win/ChangeLog

@@ +1 @@
+2011-04-11  Alexey Proskuryakov  <ap@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        WebKit2: Cannot use Ctrl-Delete as a custom keyboard shortcut
+        https://bugs.webkit.org/show_bug.cgi?id=58265
+        <rdar://problem/9221468>
+
+        * WebView.cpp: (WebView::keyDown): Removed special handling of Backspace, which is unnecessary
+        now that WebCore implements it.
+
 2011-04-06  Brian Weinstein  <bweinstein@apple.com>
 

trunk/Source/WebKit/win/WebView.cpp

@@ -1954 +1954 @@
     }
 
-    // We need to handle back/forward using either Backspace(+Shift) or Ctrl+Left/Right Arrow keys.
-    if ((virtualKeyCode == VK_BACK && keyEvent.shiftKey()) || (virtualKeyCode == VK_RIGHT && keyEvent.ctrlKey()))
+    // We need to handle back/forward using either Ctrl+Left/Right Arrow keys.
+    // FIXME: This logic should probably be in EventHandler::defaultArrowEventHandler().
+    // FIXME: Should check that other modifiers aren't pressed.
+    if (virtualKeyCode == VK_RIGHT && keyEvent.ctrlKey())
         return m_page->goForward();
-    if (virtualKeyCode == VK_BACK || (virtualKeyCode == VK_LEFT && keyEvent.ctrlKey()))
+    if (virtualKeyCode == VK_LEFT && keyEvent.ctrlKey())
         return m_page->goBack();
 

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2011-04-11  Alexey Proskuryakov  <ap@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        WebKit2: Cannot use Ctrl-Delete as a custom keyboard shortcut
+        https://bugs.webkit.org/show_bug.cgi?id=58265
+        <rdar://problem/9221468>
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::keyEvent): Added a comment explaining that doing work after DOM event
+        dispatch isn't great.
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::performNonEditingBehaviorForSelector): Added a comment explaining how this
+        might be moved down to WebCore.
+        (WebKit::WebPage::performDefaultBehaviorForKeyEvent): This is now empty, since both Space
+        and Backspace behaviors are implemented in WebCore.
+
 2011-04-11  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

@@ -957 +957 @@
 
     bool handled = handleKeyEvent(keyboardEvent, m_page.get());
+    // FIXME: Platform default behaviors should be performed during normal DOM event dispatch (in most cases, in default keydown event handler).
     if (!handled)
         handled = performDefaultBehaviorForKeyEvent(keyboardEvent);

trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

@@ -543 +543 @@
 bool WebPage::performNonEditingBehaviorForSelector(const String& selector)
 {
+    // FIXME: All these selectors have corresponding Editor commands, but the commands only work in editable content.
+    // Should such non-editing behaviors be implemented in Editor or EventHandler::defaultArrowEventHandler() perhaps?
     if (selector == "moveUp:")
         scroll(m_page.get(), ScrollUp, ScrollByLine);
@@ -575 +577 @@
 }
 
-bool WebPage::performDefaultBehaviorForKeyEvent(const WebKeyboardEvent& keyboardEvent)
-{
-    if (keyboardEvent.type() != WebEvent::KeyDown)
-        return false;
-
-    switch (keyboardEvent.windowsVirtualKeyCode()) {
-    case VK_BACK:
-        // FIXME: Handling Backspace here means that a keypress DOM event will be dispatched when focus
-        // is outside editable content, which is likely wrong. It should be moved to performNonEditingBehaviorForSelector(),
-        // which is complicated by the fact that both Backspace and Shift+Backspace get the same command.
-        if (keyboardEvent.shiftKey())
-            m_page->goForward();
-        else
-            m_page->goBack();
-        break;
-    case VK_SPACE:
-        // Space is not translated to a command by key bindings, so we need to handle it here.
-        if (keyboardEvent.shiftKey())
-            logicalScroll(m_page.get(), ScrollBlockDirectionBackward, ScrollByPage);
-        else
-            logicalScroll(m_page.get(), ScrollBlockDirectionForward, ScrollByPage);
-        break;
-    default:
-        return false;
-    }
-
-    return true;
+bool WebPage::performDefaultBehaviorForKeyEvent(const WebKeyboardEvent&)
+{
+    return false;
 }