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.

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

Check that this test change is intended #759

Closed
domenic opened this issue May 12, 2019 · 1 comment
Closed

Check that this test change is intended #759

domenic opened this issue May 12, 2019 · 1 comment

Comments

@domenic
Copy link
Member

domenic commented May 12, 2019

When implementing #750 in jsdom (jsdom/jsdom#2536), we had to change the following test, which I believe originates from some old DOM level 2 test suite.

   specify('stopPropagation should not prevent listeners on the same element from receiving the event', () => {
      this.win.addEventListener("foo", this.monitor.handleEvent, false);
      this.body.addEventListener("foo", this.monitor.handleEvent, false);
      this.plist.addEventListener("foo", this._handleEvent('stopPropagation'), true);
      this.plist.addEventListener("foo", this._handleEvent('stopPropagation'), false);
      this.plist.addEventListener("foo",  this.monitor.handleEvent, true);
      this.plist.addEventListener("foo",  this.monitor.handleEvent, false);
      this.plist.dispatchEvent(this.event);
-     assert.equal(this.monitor.atEvents.length, 4, 'should be at 4 events');
+     assert.equal(this.monitor.atEvents.length, 2, 'should be at 2 events'); // Changed from 4 to 2
      assert.equal(this.monitor.bubbledEvents.length, 0, 'should have no bubbled events');
      assert.equal(this.monitor.capturedEvents.length, 0, 'should have no captured events');
    });

The atEvents.length will increase every time the event is in the AT_TARGET phase. (Code link.)

A lot of thought seems to have gone into the recent event changes, so this is probably fine. But I wanted to highlight this in case anyone with more knowledge had concerns. /cc @rniwa.

@annevk
Copy link
Member

annevk commented May 13, 2019

I suspect what's going on here is that stopPropagation() now affects listeners on the same target, due to us copying the listeners for capture and bubbling separately there. So yeah, this seems as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants