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