Add tests, address PR feedback, refactor

jsdom · Mar 9, 2023 · 2db706a · 2db706a
1 parent f40268d
commit 2db706a
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 47 deletions.
diff --git a/lib/jsdom/living/events/Event-impl.js b/lib/jsdom/living/events/Event-impl.js
@@ -22,7 +22,7 @@ class EventImpl {
       }
     }
 
-    this.target = null;
+    this._target = null;
     this.currentTarget = null;
     this.eventPhase = 0;
 
@@ -46,6 +46,14 @@ class EventImpl {
     }
   }
 
+  get target() {
+    return this._target;
+  }
+
+  set target(target) {
+    this._target = target;
+  }
+
   get srcElement() {
     return this.target;
   }

diff --git a/lib/jsdom/living/events/MouseEvent-impl.js b/lib/jsdom/living/events/MouseEvent-impl.js
@@ -16,41 +16,50 @@ class MouseEventImpl extends UIEventImpl {
   }
   get pageX() {
     if (this._dispatchFlag) {
-      return this._initialContainingBlockRelativeX;
+      return this._pageX;
     }
     const offset = wrapperForImpl(this.view)?.scrollX || 0;
     return offset + this.clientX;
   }
   get pageY() {
     if (this._dispatchFlag) {
-      return this._initialContainingBlockRelativeY;
+      return this._pageY;
     }
     const offset = wrapperForImpl(this.view)?.scrollY || 0;
     return offset + this.clientY;
   }
   get offsetX() {
     if (this._dispatchFlag) {
-      return this._paddingEdgeRelativeX;
+      return this._offsetX;
     }
     return this.pageX;
   }
   get offsetY() {
     if (this._dispatchFlag) {
-      return this._paddingEdgeRelativeY;
+      return this._offsetY;
     }
     return this.pageY;
   }
 
-  constructor(globalObject, args, privateData) {
-    super(globalObject, args, privateData);
+  /** @override */
+  get target() {
+    return super.target;
+  }
 
-    // We cache this during init so that we have a stable value during dispatch
-    const { scrollX = 0, scrollY = 0 } = wrapperForImpl(this.view) ?? {};
-    this._initialContainingBlockRelativeX = this.clientX + scrollX;
-    this._initialContainingBlockRelativeY = this.clientY + scrollY;
-    // TODO: support non-zero values somehow if/when jsdom adds layout support
-    this._paddingEdgeRelativeX = 0;
-    this._paddingEdgeRelativeY = 0;
+  /**
+   * @override
+   * calculate the event position when the target is set so that it's stable during dispatch
+   */
+  set target(target) {
+    super.target = target;
+    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(

diff --git a/...atform-tests/to-upstream/html/editing/activation/click-event-properties-dontupstream.html b/...atform-tests/to-upstream/html/editing/activation/click-event-properties-dontupstream.html
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>click() should do nothing on disabled elements</title>
+<link rel="author" title="Domenic Denicola" href="mailto:d@domenic.me">
+<link rel="help" href="https://html.spec.whatwg.org/multipage/#attr-button-type-submit-state">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<button>Click me</button>
+
+<script>
+"use strict";
+
+// these tests should not be upstreamed since:
+// 1. we're testing pageX/Y and scroll offsets by setting scrollX/Y which should be readonly; jsdom doesn't implement
+//    scroll() and friends (yet)
+// 2. getBoundingClientRect() doesn't know about layout/scrolling, so jsdom can return some unusual offsetX/Y values
+//    during dispatch despite the calculation logic being correct (see below)
+// 3. equivalent tests are being added upstream in https://github.com/web-platform-tests/wpt/pull/38912 that use scroll()
+//    and expect correct offsetX/Y values during dispatch
+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);
+}, "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 => {
+    // changing these during dispatch should not mess up pageX/Y
+    window.scrollX = 0;
+    window.scrollY = 0;
+    // coordinate reflects the right values as of the start of dispatch 👍
+    assert_equals(e.pageX, 110);
+    assert_equals(e.pageY, 220);
+    // these values are slightly wonky if there is a scroll offset, since getBoundingClientRect doesn't (yet)
+    // consider scrolling or layout
+    assert_equals(e.offsetX, 100);
+    assert_equals(e.offsetY, 200);
+  }));
+  window.scrollX = 10;
+  window.scrollY = 20;
+  element.dispatchEvent(new MouseEvent("click", {
+    clientX: 100, clientY: 200, view: window
+  }));
+}, "MouseEvent should provide the correct computed values when the dispatch flag is set");
+</script>
diff --git a/test/web-platform-tests/to-upstream/html/editing/activation/click-event-properties.html b/test/web-platform-tests/to-upstream/html/editing/activation/click-event-properties.html
@@ -90,38 +90,6 @@
 
 }, "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);
-}, "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 => {
-    // changing these during dispatch should not mess up pageX/Y
-    window.scrollX = 0;
-    window.scrollY = 0;
-    assert_equals(e.pageX, 110);
-    assert_equals(e.pageY, 220);
-    assert_equals(e.offsetX, 0);
-    assert_equals(e.offsetY, 0);
-  }));
-  window.scrollX = 10;
-  window.scrollY = 20;
-  element.dispatchEvent(new MouseEvent("click", {
-    clientX: 100, clientY: 200, view: window
-  }));
-}, "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
@@ -139,6 +107,5 @@
   assert_equals(e.screenY, 2);
   assert_equals(e.clientX, 3);
   assert_equals(e.clientY, 4);
-
 }, "MouseEvent.initMouseEvent should convert doubles to longs");
 </script>