events: add stop propagation flag to Event.stopImmediatePropagation by mikemadest · Pull Request #39463 · nodejs/node

mikemadest · 2021-07-20T08:02:57Z

Spec mention stopImmediatePropagation should set both flags:
"stop propagation" and "stop immediate propagation".

So the second is not supported by Node as there is no hierarchy and bubbling,
but the flags are both present as well as stopPropagation.

Would it make sense to follow specs on that?
Refs: https://dom.spec.whatwg.org/#dom-event-stopimmediatepropagation

mikemadest · 2021-07-20T12:00:05Z

I didn't want to make a description too long, but feel like a bit more context could be interesting:
As part of contributing to linkedom (https://github.com/WebReflection/linkedom), I was adding event bubbling there.

When Event and EventTarget are available in Node js, the choice was to extend them to provide what was needed (bubbling in dispatchEvent).
Since node don't need the bubbling part it was expected
(I had a good read at #33556 (comment)).

But since Event had everything needed I didn't think it would be required there, except for that tiny missing line in stopImmediatePropagation and it felt like maybe this should be here?
Hoping I don't waste anybody's time here, I understand it's not game changer.

mikemadest · 2023-06-26T08:31:36Z

Should this be closed?
This was approved but now is quite old and I'm not sure if there is any interest in merging it. I solved the conflicts just in case...

benjamingr · 2023-06-26T09:21:08Z

No this fix looks correct it was just missed - apologies this is our bad.

nodejs-github-bot · 2023-06-26T09:24:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/52488/

mikemadest · 2023-06-26T13:21:50Z

No this fix looks correct it was just missed - apologies this is our bad.

No worries, thank you for the answer!
There seems to be errors on the tests, I'll check that and will be active on this so if you don't see any changes, I'm probably working on them or trying to understand what's wrong.

mikemadest · 2023-06-27T11:05:13Z

Pull request updated:
solved conflicts, rebased, tested (everything passed) and pushed again. Everything should be fine now.

benjamingr · 2023-06-27T12:32:48Z

Thank you for your patience we truly don't deserve it and apologies for the contribution experience.

Let's run CI and land this as soon as it's green.

nodejs-github-bot · 2023-06-27T12:47:14Z

CI: https://ci.nodejs.org/job/node-test-pull-request/52515/

nodejs-github-bot · 2023-08-10T09:27:41Z

CI: https://ci.nodejs.org/job/node-test-pull-request/53118/

nodejs-github-bot · 2023-08-14T09:35:27Z

CI: https://ci.nodejs.org/job/node-test-pull-request/53301/

nodejs-github-bot · 2023-08-14T18:39:23Z

Commit Queue failed

- Loading data for nodejs/node/pull/39463
✔ Done loading data for nodejs/node/pull/39463
----------------------------------- PR info ------------------------------------
Title events: add stop propagation flag to Event.stopImmediatePropagation (#39463)
Author Mickael Meausoone (@mikemadest, first-time contributor)
Branch mikemadest:stop-immediate-propagation-flags -> nodejs:main
Labels needs-ci
Commits 1
- events: add stop propagation flag to Event.stopImmediatePropagation
Committers 1
- Mickael Meausoone PR-URL: https://github.com/nodejs/node/pull/39463
Refs: https://dom.spec.whatwg.org/#dom-event-stopimmediatepropagation
Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/39463
Refs: https://dom.spec.whatwg.org/#dom-event-stopimmediatepropagation
Reviewed-By: James M Snell Reviewed-By: Benjamin Gruenbaum --------------------------------------------------------------------------------
⚠ Commits were pushed since the last approving review:
⚠ - events: add stop propagation flag to Event.stopImmediatePropagation
ℹ This PR was created on Tue, 20 Jul 2021 08:02:57 GMT
✔ Approvals: 2
✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/39463#pullrequestreview-710658397
✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/39463#pullrequestreview-1569362103
✘ Last GitHub CI failed
ℹ Last Full PR CI on 2023-08-14T09:35:27Z: https://ci.nodejs.org/job/node-test-pull-request/53301/
- Querying data for job/node-test-pull-request/53301/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/5859369572

Spec mention stopImmediatePropagation should set both flags: "stop propagation" and "stop immediate propagation". So the second is not supported by Node as there is no hierarchy and bubbling, but the flags are both present as well as stopPropagation. It would makes sense to follow specs on that. Refs: https://dom.spec.whatwg.org/#dom-event-stopimmediatepropagation

nodejs-github-bot · 2024-05-11T23:01:09Z

CI: https://ci.nodejs.org/job/node-test-pull-request/59127/

nodejs-github-bot · 2024-05-12T05:18:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/59135/

aduh95 · 2024-05-12T07:18:04Z

Landed in 04cf8c2

Spec mention stopImmediatePropagation should set both flags: "stop propagation" and "stop immediate propagation". So the second is not supported by Node.js as there is no hierarchy and bubbling, but the flags are both present as well as stopPropagation. It would makes sense to follow specs on that. Refs: https://dom.spec.whatwg.org/#dom-event-stopimmediatepropagation PR-URL: #39463 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Spec mention stopImmediatePropagation should set both flags: "stop propagation" and "stop immediate propagation". So the second is not supported by Node.js as there is no hierarchy and bubbling, but the flags are both present as well as stopPropagation. It would makes sense to follow specs on that. Refs: https://dom.spec.whatwg.org/#dom-event-stopimmediatepropagation PR-URL: nodejs#39463 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jul 20, 2021

mikemadest force-pushed the stop-immediate-propagation-flags branch from 33c4ebc to 23744d2Compare July 20, 2021 08:15

mikemadest changed the title ~~Events: add stop propagation flag to Event.stopImmediatePropagation~~events: add stop propagation flag to Event.stopImmediatePropagationJul 20, 2021

jasnell approved these changes Jul 20, 2021
View reviewed changes

mikemadest mentioned this pull request Jul 20, 2021
Extend EventTarget to support event bubbling WebReflection/linkedom#84
Merged

Trott force-pushed the main branch from 2d76238 to ca3ed36Compare November 12, 2022 01:49

benjamingr approved these changes Jun 26, 2023
View reviewed changes

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2023

mikemadest force-pushed the stop-immediate-propagation-flags branch from 527ff4d to 53f073dCompare June 27, 2023 11:00

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2023

mikemadest force-pushed the stop-immediate-propagation-flags branch from 53f073d to d7de14dCompare July 2, 2023 07:55

This was referenced Jul 3, 2023
CI Reliability 2023-07-03 nodejs/reliability#606
Open
CI Reliability 2023-07-04 nodejs/reliability#607
Open

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 4, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 4, 2023

mikemadest force-pushed the stop-immediate-propagation-flags branch from c4739fa to d2b2e2cCompare August 8, 2023 12:58

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 9, 2023

benjamingr approved these changes Aug 9, 2023
View reviewed changes

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 10, 2023

mikemadest force-pushed the stop-immediate-propagation-flags branch from d2b2e2c to d712a39Compare August 14, 2023 07:53

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2023

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2023

benjamingr added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 14, 2023

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 14, 2023

aduh95 force-pushed the stop-immediate-propagation-flags branch from d712a39 to 991ae2eCompare May 11, 2024 17:54

aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels May 11, 2024

aduh95 merged commit 04cf8c2 into nodejs:mainMay 12, 2024

targos mentioned this pull request May 13, 2024
v22.2.0 release proposal #52971
Merged

marco-ippolito mentioned this pull request Jun 17, 2024
v20.15.0 proposal #53486
Merged

Uh oh!

Uh oh!

Conversation

mikemadest commented Jul 20, 2021

Uh oh!

mikemadest commented Jul 20, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikemadest commented Jun 26, 2023

Uh oh!

benjamingr commented Jun 26, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 26, 2023

Uh oh!

mikemadest commented Jun 26, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikemadest commented Jun 27, 2023

Uh oh!

benjamingr commented Jun 27, 2023

Uh oh!

nodejs-github-bot commented Jun 27, 2023

Uh oh!

nodejs-github-bot commented Aug 10, 2023

Uh oh!

nodejs-github-bot commented Aug 14, 2023

Uh oh!

nodejs-github-bot commented Aug 14, 2023

Uh oh!

nodejs-github-bot commented May 11, 2024

Uh oh!

nodejs-github-bot commented May 12, 2024

Uh oh!

aduh95 commented May 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mikemadest commented Jul 20, 2021•
edited
Loading

benjamingr commented Jun 26, 2023•
edited
Loading

mikemadest commented Jun 26, 2023•
edited
Loading