Title: [284146] trunk/Source/WebCore
Revision
284146
Author
[email protected]
Date
2021-10-13 21:09:59 -0700 (Wed, 13 Oct 2021)

Log Message

Use PlatformKeyboardEvent in KeyboardScrollingAnimator to fix a layering violation
https://bugs.webkit.org/show_bug.cgi?id=231711

Reviewed by Beth Dakin.

KeyboardScrollingAnimator lives in platform/ so should not know about dom/KeyboardEvent.
Have it use PlatformKeyboardEvent instead.

* page/EventHandler.cpp:
(WebCore::EventHandler::stopKeyboardScrolling):
(WebCore::EventHandler::startKeyboardScrolling): Null check the view. Use the platform event.
* platform/KeyboardScrollingAnimator.cpp:
(WebCore::keyboardScrollingKeyFromEvent):
(WebCore::KeyboardScrollingAnimator::keyboardScrollForKeyboardEvent const):
(WebCore::KeyboardScrollingAnimator::beginKeyboardScrollGesture):
* platform/KeyboardScrollingAnimator.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (284145 => 284146)


--- trunk/Source/WebCore/ChangeLog	2021-10-14 03:58:16 UTC (rev 284145)
+++ trunk/Source/WebCore/ChangeLog	2021-10-14 04:09:59 UTC (rev 284146)
@@ -1,3 +1,22 @@
+2021-10-13  Simon Fraser  <[email protected]>
+
+        Use PlatformKeyboardEvent in KeyboardScrollingAnimator to fix a layering violation
+        https://bugs.webkit.org/show_bug.cgi?id=231711
+
+        Reviewed by Beth Dakin.
+
+        KeyboardScrollingAnimator lives in platform/ so should not know about dom/KeyboardEvent.
+        Have it use PlatformKeyboardEvent instead.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::stopKeyboardScrolling):
+        (WebCore::EventHandler::startKeyboardScrolling): Null check the view. Use the platform event.
+        * platform/KeyboardScrollingAnimator.cpp:
+        (WebCore::keyboardScrollingKeyFromEvent):
+        (WebCore::KeyboardScrollingAnimator::keyboardScrollForKeyboardEvent const):
+        (WebCore::KeyboardScrollingAnimator::beginKeyboardScrollGesture):
+        * platform/KeyboardScrollingAnimator.h:
+
 2021-10-13  Megan Gardner  <[email protected]>
 
         Scroll To Text Fragment directive parsing

Modified: trunk/Source/WebCore/page/EventHandler.cpp (284145 => 284146)


--- trunk/Source/WebCore/page/EventHandler.cpp	2021-10-14 03:58:16 UTC (rev 284145)
+++ trunk/Source/WebCore/page/EventHandler.cpp	2021-10-14 04:09:59 UTC (rev 284146)
@@ -4321,10 +4321,11 @@
 void EventHandler::stopKeyboardScrolling()
 {
     Ref protectedFrame = m_frame;
-    FrameView* view = m_frame.view();
+    auto* view = m_frame.view();
+    if (!view)
+        return;
 
-    KeyboardScrollingAnimator* animator = view->scrollAnimator().keyboardScrollingAnimator();
-
+    auto* animator = view->scrollAnimator().keyboardScrollingAnimator();
     if (animator)
         animator->handleKeyUpEvent();
 }
@@ -4335,13 +4336,15 @@
         return false;
 
     Ref protectedFrame = m_frame;
-    FrameView* view = m_frame.view();
+    auto* view = m_frame.view();
+    if (!view)
+        return false;
 
-    KeyboardScrollingAnimator* animator = view->scrollAnimator().keyboardScrollingAnimator();
+    auto* animator = view->scrollAnimator().keyboardScrollingAnimator();
+    auto* platformEvent = event.underlyingPlatformEvent();
+    if (animator && platformEvent)
+        return animator->beginKeyboardScrollGesture(*platformEvent);
 
-    if (animator)
-        return animator->beginKeyboardScrollGesture(event);
-
     return false;
 }
 

Modified: trunk/Source/WebCore/platform/KeyboardScrollingAnimator.cpp (284145 => 284146)


--- trunk/Source/WebCore/platform/KeyboardScrollingAnimator.cpp	2021-10-14 03:58:16 UTC (rev 284145)
+++ trunk/Source/WebCore/platform/KeyboardScrollingAnimator.cpp	2021-10-14 04:09:59 UTC (rev 284146)
@@ -27,6 +27,7 @@
 #include "KeyboardScrollingAnimator.h"
 
 #include "EventNames.h"
+#include "PlatformKeyboardEvent.h"
 #include "ScrollTypes.h"
 #include "ScrollableArea.h"
 #include "WritingMode.h"
@@ -33,6 +34,16 @@
 
 namespace WebCore {
 
+enum class KeyboardScrollingKey : uint8_t {
+    LeftArrow,
+    RightArrow,
+    UpArrow,
+    DownArrow,
+    Space,
+    PageUp,
+    PageDown
+};
+
 KeyboardScrollingAnimator::KeyboardScrollingAnimator(ScrollAnimator& scrollAnimator, ScrollingEffectsController& scrollController)
     : m_scrollAnimator(scrollAnimator)
     , m_scrollController(scrollController)
@@ -158,45 +169,50 @@
     return 0;
 }
 
-std::optional<KeyboardScroll> KeyboardScrollingAnimator::keyboardScrollForKeyboardEvent(KeyboardEvent& event) const
+static std::optional<KeyboardScrollingKey> keyboardScrollingKeyFromEvent(const PlatformKeyboardEvent& event)
 {
-    // FIXME (bug 227459): This logic does not account for writing-mode.
+    auto identifier = event.keyIdentifier();
+    if (identifier == "Left")
+        return KeyboardScrollingKey::LeftArrow;
+    if (identifier == "Right")
+        return KeyboardScrollingKey::RightArrow;
+    if (identifier == "Up")
+        return KeyboardScrollingKey::UpArrow;
+    if (identifier == "Down")
+        return KeyboardScrollingKey::DownArrow;
+    if (identifier == "PageUp")
+        return KeyboardScrollingKey::PageUp;
+    if (identifier == "PageDown")
+        return KeyboardScrollingKey::PageDown;
 
-    enum class Key : uint8_t { LeftArrow, RightArrow, UpArrow, DownArrow, Space, PageUp, PageDown };
+    if (event.text().characterStartingAt(0) == ' ')
+        return KeyboardScrollingKey::Space;
 
-    Key key;
-    if (event.keyIdentifier() == "Left")
-        key = Key::LeftArrow;
-    else if (event.keyIdentifier() == "Right")
-        key = Key::RightArrow;
-    else if (event.keyIdentifier() == "Up")
-        key = Key::UpArrow;
-    else if (event.keyIdentifier() == "Down")
-        key = Key::DownArrow;
-    else if (event.charCode() == ' ')
-        key = Key::Space;
-    else if (event.keyIdentifier() == "PageUp")
-        key = Key::PageUp;
-    else if (event.keyIdentifier() == "PageDown")
-        key = Key::PageDown;
-    else
-        return std::nullopt;
+    return { };
+}
 
+std::optional<KeyboardScroll> KeyboardScrollingAnimator::keyboardScrollForKeyboardEvent(const PlatformKeyboardEvent& event) const
+{
+    auto key = keyboardScrollingKeyFromEvent(event);
+    if (!key)
+        return { };
+
+    // FIXME (bug 227459): This logic does not account for writing-mode.
     auto granularity = [&] {
-        switch (key) {
-        case Key::LeftArrow:
-        case Key::RightArrow:
+        switch (key.value()) {
+        case KeyboardScrollingKey::LeftArrow:
+        case KeyboardScrollingKey::RightArrow:
             return event.altKey() ? ScrollGranularity::ScrollByPage : ScrollGranularity::ScrollByLine;
-        case Key::UpArrow:
-        case Key::DownArrow:
+        case KeyboardScrollingKey::UpArrow:
+        case KeyboardScrollingKey::DownArrow:
             if (event.metaKey())
                 return ScrollGranularity::ScrollByDocument;
             if (event.altKey())
                 return ScrollGranularity::ScrollByPage;
             return ScrollGranularity::ScrollByLine;
-        case Key::Space:
-        case Key::PageUp:
-        case Key::PageDown:
+        case KeyboardScrollingKey::Space:
+        case KeyboardScrollingKey::PageUp:
+        case KeyboardScrollingKey::PageDown:
             return ScrollGranularity::ScrollByPage;
         };
         RELEASE_ASSERT_NOT_REACHED();
@@ -203,18 +219,18 @@
     }();
 
     auto direction = [&] {
-        switch (key) {
-        case Key::LeftArrow:
+        switch (key.value()) {
+        case KeyboardScrollingKey::LeftArrow:
             return ScrollDirection::ScrollLeft;
-        case Key::RightArrow:
+        case KeyboardScrollingKey::RightArrow:
             return ScrollDirection::ScrollRight;
-        case Key::UpArrow:
-        case Key::PageUp:
+        case KeyboardScrollingKey::UpArrow:
+        case KeyboardScrollingKey::PageUp:
             return ScrollDirection::ScrollUp;
-        case Key::DownArrow:
-        case Key::PageDown:
+        case KeyboardScrollingKey::DownArrow:
+        case KeyboardScrollingKey::PageDown:
             return ScrollDirection::ScrollDown;
-        case Key::Space:
+        case KeyboardScrollingKey::Space:
             return event.shiftKey() ? ScrollDirection::ScrollUp : ScrollDirection::ScrollDown;
         }
         RELEASE_ASSERT_NOT_REACHED();
@@ -236,16 +252,16 @@
     return scroll;
 }
 
-bool KeyboardScrollingAnimator::beginKeyboardScrollGesture(KeyboardEvent& event)
+bool KeyboardScrollingAnimator::beginKeyboardScrollGesture(const PlatformKeyboardEvent& event)
 {
     auto scroll = keyboardScrollForKeyboardEvent(event);
-
     if (!scroll)
         return false;
 
     m_currentKeyboardScroll = scroll;
 
-    if (!(event.type() == eventNames().keydownEvent || event.type() == eventNames().keypressEvent))
+    // PlatformEvent::Char is a "keypress" event.
+    if (!(event.type() == PlatformEvent::RawKeyDown || event.type() == PlatformEvent::Char))
         return false;
 
     if (m_scrollTriggeringKeyIsPressed)

Modified: trunk/Source/WebCore/platform/KeyboardScrollingAnimator.h (284145 => 284146)


--- trunk/Source/WebCore/platform/KeyboardScrollingAnimator.h	2021-10-14 03:58:16 UTC (rev 284145)
+++ trunk/Source/WebCore/platform/KeyboardScrollingAnimator.h	2021-10-14 04:09:59 UTC (rev 284146)
@@ -25,7 +25,6 @@
 
 #pragma once
 
-#include "KeyboardEvent.h" // FIXME: This is a layering violation.
 #include "KeyboardScroll.h" // FIXME: This is a layering violation.
 #include "RectEdges.h"
 #include "ScrollAnimator.h"
@@ -32,6 +31,8 @@
 
 namespace WebCore {
 
+class PlatformKeyboardEvent;
+
 class KeyboardScrollingAnimator {
     WTF_MAKE_NONCOPYABLE(KeyboardScrollingAnimator);
     WTF_MAKE_FAST_ALLOCATED;
@@ -38,7 +39,7 @@
 public:
     KeyboardScrollingAnimator(ScrollAnimator&, ScrollingEffectsController&);
 
-    bool beginKeyboardScrollGesture(KeyboardEvent&);
+    bool beginKeyboardScrollGesture(const PlatformKeyboardEvent&);
     void handleKeyUpEvent();
     void updateKeyboardScrollPosition(MonotonicTime);
 
@@ -45,7 +46,7 @@
 private:
     void stopKeyboardScrollAnimation();
     RectEdges<bool> scrollableDirectionsFromPosition(FloatPoint) const;
-    std::optional<KeyboardScroll> keyboardScrollForKeyboardEvent(KeyboardEvent&) const;
+    std::optional<KeyboardScroll> keyboardScrollForKeyboardEvent(const PlatformKeyboardEvent&) const;
     float scrollDistance(ScrollDirection, ScrollGranularity) const;
 
     ScrollAnimator& m_scrollAnimator;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to