Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MouseEvent: fix pageX/Y and offsetX/Y during dispatch #3514

Merged
merged 6 commits into from Apr 16, 2023
Merged

MouseEvent: fix pageX/Y and offsetX/Y during dispatch #3514

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
23ad658
MouseEvent: fix `pageX/Y` during dispatch
jenseng Mar 2, 2023
f183b3d
Add tests, address PR feedback, refactor
jenseng Mar 9, 2023
263d7b2
Remove scrollX/Y setting from tests
jenseng Mar 12, 2023
5248f0b
Incorporate _receivedTarget suggestion
jenseng Mar 14, 2023
795f81e
Address PR feedback
jenseng Apr 5, 2023
9fd7bc4
Tweak comments
jenseng Apr 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/jsdom/living/events/Event-impl.js
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@ class EventImpl {
}
}

this.target = null;
this._target = null;
this.currentTarget = null;
this.eventPhase = 0;

Expand All @@ -46,6 +46,14 @@ class EventImpl {
}
}

get target() {
return this._target;
}

set target(target) {
this._target = target;
}

get srcElement() {
return this.target;
}
Expand Down
25 changes: 21 additions & 4 deletions lib/jsdom/living/events/MouseEvent-impl.js
View file
Open in desktop
Expand Up @@ -16,31 +16,48 @@ class MouseEventImpl extends UIEventImpl {
}
get pageX() {
if (this._dispatchFlag) {
return 0;
return this._pageX;
}
const offset = wrapperForImpl(this.view)?.scrollX || 0;
return offset + this.clientX;
}
get pageY() {
if (this._dispatchFlag) {
return 0;
return this._pageY;
}
const offset = wrapperForImpl(this.view)?.scrollY || 0;
return offset + this.clientY;
}
get offsetX() {
if (this._dispatchFlag) {
return 0;
return this._offsetX;
}
return this.pageX;
}
get offsetY() {
if (this._dispatchFlag) {
return 0;
return this._offsetY;
}
return this.pageY;
}

get target() {
return super.target;
}

set target(target) {
super.target = target;
Copy link
Contributor

@jsnajdr jsnajdr Mar 13, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to implement this, maybe more elegant, would be a _receivedTarget method on the parent EventImpl class:

class EventImpl {
  set target(target) {
    this._target = target;
    this._receivedTarget();
  }

  _receivedTarget() {}
}

class MouseEventImpl {
  _receivedTarget() {
    // calculate offsets from this.target
  }
}

Chrome's Event implementation also has a ::ReceivedTarget method, and JSDOM also has similar prior art in the _activationBehavior() method on elements.

// calculate the event position when the target is set so that it's stable during dispatch
if (target) {
const { scrollX = 0, scrollY = 0 } = wrapperForImpl(this.view) ?? {};
this._pageX = this.clientX + scrollX;
this._pageY = this.clientY + scrollY;
const domRect = target.getBoundingClientRect?.() ?? { x: 0, y: 0 };
this._offsetX = this.clientX - domRect.x;
this._offsetY = this.clientY - domRect.y;
}
}

initMouseEvent(
type,
bubbles,
Expand Down
24 changes: 17 additions & 7 deletions test/web-platform-tests/to-upstream/html/editing/activation/click-event-properties.html
View file
Open in desktop
Expand Up @@ -91,19 +91,30 @@
}, "clicking using click() should produce an event object with the correct MouseEventInit values");

test(() => {
window.scrollX = 10;
window.scrollY = 20;
const e = new MouseEvent("click", {
clientX: 1, clientY: 2, view: window
});
assert_equals(e.x, 1);
assert_equals(e.y, 2);
assert_equals(e.pageX, 11);
assert_equals(e.pageY, 22);
assert_equals(e.offsetX, 11);
assert_equals(e.offsetY, 22);
assert_equals(e.pageX, 1);
assert_equals(e.pageY, 2);
assert_equals(e.offsetX, 1);
assert_equals(e.offsetY, 2);
}, "MouseEvent should provide the correct computed values when the dispatch flag is not set");

async_test(t => {
const element = document.querySelector("button");
element.addEventListener("click", t.step_func_done(e => {
assert_equals(e.pageX, 1);
assert_equals(e.pageY, 2);
assert_equals(e.offsetX, 1);
assert_equals(e.offsetY, 2);
}));
element.dispatchEvent(new MouseEvent("click", {
clientX: 1, clientY: 2, view: window
}));
jenseng marked this conversation as resolved.
Show resolved Hide resolved
}, "MouseEvent should provide the correct computed values when the dispatch flag is set");

test(() => {
const e = new MouseEvent("click", {
screenX: 1.5, screenY: 2.5, clientX: 3.5, clientY: 4.5
Expand All @@ -121,6 +132,5 @@
assert_equals(e.screenY, 2);
assert_equals(e.clientX, 3);
assert_equals(e.clientY, 4);

}, "MouseEvent.initMouseEvent should convert doubles to longs");
</script>