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:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@284146 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e39fa05..981ae0b 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2021-10-13 Simon Fraser <simon.fraser@apple.com>
+
+ 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 <megan_gardner@apple.com>
Scroll To Text Fragment directive parsing
diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
index 46d9ff8..61935b3 100644
--- a/Source/WebCore/page/EventHandler.cpp
+++ b/Source/WebCore/page/EventHandler.cpp
@@ -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,12 +4336,14 @@
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();
-
- if (animator)
- return animator->beginKeyboardScrollGesture(event);
+ auto* animator = view->scrollAnimator().keyboardScrollingAnimator();
+ auto* platformEvent = event.underlyingPlatformEvent();
+ if (animator && platformEvent)
+ return animator->beginKeyboardScrollGesture(*platformEvent);
return false;
}
diff --git a/Source/WebCore/platform/KeyboardScrollingAnimator.cpp b/Source/WebCore/platform/KeyboardScrollingAnimator.cpp
index fd8f0fc..cacd043 100644
--- a/Source/WebCore/platform/KeyboardScrollingAnimator.cpp
+++ b/Source/WebCore/platform/KeyboardScrollingAnimator.cpp
@@ -27,12 +27,23 @@
#include "KeyboardScrollingAnimator.h"
#include "EventNames.h"
+#include "PlatformKeyboardEvent.h"
#include "ScrollTypes.h"
#include "ScrollableArea.h"
#include "WritingMode.h"
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,63 +169,68 @@
return 0;
}
-std::optional<KeyboardScroll> KeyboardScrollingAnimator::keyboardScrollForKeyboardEvent(KeyboardEvent& event) const
+static std::optional<KeyboardScrollingKey> keyboardScrollingKeyFromEvent(const PlatformKeyboardEvent& event)
{
+ 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;
+
+ if (event.text().characterStartingAt(0) == ' ')
+ return KeyboardScrollingKey::Space;
+
+ 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.
-
- enum class Key : uint8_t { LeftArrow, RightArrow, UpArrow, DownArrow, Space, PageUp, PageDown };
-
- 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;
-
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();
}();
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)
diff --git a/Source/WebCore/platform/KeyboardScrollingAnimator.h b/Source/WebCore/platform/KeyboardScrollingAnimator.h
index 3c9814f..485abb0 100644
--- a/Source/WebCore/platform/KeyboardScrollingAnimator.h
+++ b/Source/WebCore/platform/KeyboardScrollingAnimator.h
@@ -25,27 +25,28 @@
#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"
namespace WebCore {
+class PlatformKeyboardEvent;
+
class KeyboardScrollingAnimator {
WTF_MAKE_NONCOPYABLE(KeyboardScrollingAnimator);
WTF_MAKE_FAST_ALLOCATED;
public:
KeyboardScrollingAnimator(ScrollAnimator&, ScrollingEffectsController&);
- bool beginKeyboardScrollGesture(KeyboardEvent&);
+ bool beginKeyboardScrollGesture(const PlatformKeyboardEvent&);
void handleKeyUpEvent();
void updateKeyboardScrollPosition(MonotonicTime);
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;
