Skip to content

Commit

Permalink
pointerevent_movementxy.html?mouse WPT is failing
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=258780
rdar://111635046

Reviewed by Wenson Hsieh.

Currently, we are failing the pointerevent_movementxy.html?mouse WPT
variant with the error "FAIL mouse pointerevent attributes
assert_equals: movementX should be 0 for event other than pointermove.
expected 0 but got 20". We fail this assertion because events that are
not of the pointermove type have non-zero movementX/Y values.

We address said issue in this commit by:

- Making sure that any event which isn't of type (mouse/pointer/touch)move
does not have non-zero movementX/movementY fields.
- Hardening against accidentally assigning movementX/movementY fields
outside of initialization, by making said fields private.
- Adjusting the expected result for the now passing WPT.

This commit also introduces some additional state into the
WebCore::Event class, through which we can consult if the Event was
constructed using an EventInit dictionary. We need this information
because the attribute values assigned from the EventInit dictionary
receive priority over other rules dictating the Event attribute.

* LayoutTests/imported/w3c/web-platform-tests/pointerevents/pointerevent_movementxy_mouse-expected.txt:
* Source/WebCore/dom/Event.cpp:
* Source/WebCore/dom/Event.h:
(WebCore::Event::isConstructedFromInitializer const):
* Source/WebCore/dom/MouseRelatedEvent.cpp:
(WebCore::isMoveEventType):
(WebCore::MouseRelatedEvent::init):
* Source/WebCore/dom/MouseRelatedEvent.h:

Canonical link: https://commits.webkit.org/265730@main
  • Loading branch information
aprotyas committed Jul 3, 2023
1 parent dc7f855 commit dd9d402
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
Pointer Events movementX/Y attribute test

Follow the test instructions with mouse. If you don't have the device skip it.

Test Description: This test checks the movementX/Y properties of pointer events.
Press down on the black square.
Move your pointer slowly along a straight line to the red square.
Expand All @@ -10,5 +8,5 @@ Test passes if the proper behavior of the events is observed.



FAIL mouse pointerevent attributes assert_equals: movementX should be 0 for event other than pointermove. expected 0 but got 20
PASS mouse pointerevent attributes

1 change: 1 addition & 0 deletions Source/WebCore/dom/Event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ Event::Event(const AtomString& eventType, const EventInit& initializer, IsTruste
initializer.composed ? IsComposed::Yes : IsComposed::No }
{
ASSERT(!eventType.isNull());
m_isConstructedFromInitializer = true;
}

Event::~Event() = default;
Expand Down
6 changes: 6 additions & 0 deletions Source/WebCore/dom/Event.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ class Event : public ScriptWrappable, public RefCounted<Event> {

virtual void receivedTarget() { }

bool isConstructedFromInitializer() const { return m_isConstructedFromInitializer; }

private:
explicit Event(MonotonicTime createTime, const AtomString& type, IsTrusted, CanBubble, IsCancelable, IsComposed);

Expand All @@ -181,6 +183,10 @@ class Event : public ScriptWrappable, public RefCounted<Event> {

unsigned m_eventPhase : 2;

// We consult this flag since the EventInit dictionary takes priority in initializing event attribute values.
// See step 4 of https://dom.spec.whatwg.org/#inner-event-creation-steps
unsigned m_isConstructedFromInitializer : 1 { false };

AtomString m_type;

RefPtr<EventTarget> m_currentTarget;
Expand Down
13 changes: 13 additions & 0 deletions Source/WebCore/dom/MouseRelatedEvent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ MouseRelatedEvent::MouseRelatedEvent(const AtomString& eventType, const MouseRel
init(false, IntPoint(0, 0));
}

static inline bool isMoveEventType(const AtomString& eventType)
{
auto& eventNames = WebCore::eventNames();
return eventType == eventNames.mousemoveEvent
|| eventType == eventNames.pointermoveEvent
|| eventType == eventNames.touchmoveEvent;
}

void MouseRelatedEvent::init(bool isSimulated, const IntPoint& windowLocation)
{
if (!isSimulated) {
Expand All @@ -76,6 +84,11 @@ void MouseRelatedEvent::init(bool isSimulated, const IntPoint& windowLocation)
}

initCoordinates();

if (!isConstructedFromInitializer() && !isMoveEventType(type())) {
m_movementX = 0;
m_movementY = 0;
}
}

void MouseRelatedEvent::initCoordinates()
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/dom/MouseRelatedEvent.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,12 @@ class MouseRelatedEvent : public UIEventWithKeyState {
// Expose these so MouseEvent::initMouseEvent can set them.
IntPoint m_screenLocation;
LayoutPoint m_clientLocation;
double m_movementX { 0 };
double m_movementY { 0 };

private:
void init(bool isSimulated, const IntPoint&);

double m_movementX { 0 };
double m_movementY { 0 };
LayoutPoint m_pageLocation;
LayoutPoint m_layerLocation;
LayoutPoint m_offsetLocation;
Expand Down

0 comments on commit dd9d402

Please sign in to comment.