bug(Zone.js): removing capture:true event listener breaks .on* event listeners

[maxpatiiuk]

Which @angular/* package(s) are the source of the bug?

zone.js

Is this a regression?

Yes

Description

The fix for #31643 introduced a bug

The following code

angular/packages/zone.js/lib/common/events.ts

Lines 580 to 583 in 8e65bdc

	if (typeof eventName === 'string') {
	const onPropertySymbol = ZONE_SYMBOL_PREFIX + 'ON_PROPERTY' + eventName;
	target[onPropertySymbol] = null;
	}

removes on* property listener when it's not supposed to.
This happens because the code, when checking whether there are no more "capture click" tasks remaining (existingTasks.length === 0), forgets to check whether there are any "non-capture click" tasks present
Though, since on* listeners are non-capturing, maybe this code shouldn't run at all when capture:true events are removed?

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/github-6rdbwr?file=src%2Fmain.ts

Please provide the exception or error you saw

// Make a button
const button = document.createElement('button');
document.body.append(button);

// Set onclick property
button.onclick = () => console.log('onclick!');
console.log(
  'See that onclick prop is set correctly: ',
  (button as any).__zone_symbol__ON_PROPERTYclick
);

// Some unrelated code sets and removes the event listener, WITH CAPTURE:TRUE
button.addEventListener('click', console.log, { capture: true });
button.removeEventListener('click', console.log, { capture: true });

console.log(
  'See that onclick handler was removed (BUG!): ',
  (button as any).__zone_symbol__ON_PROPERTYclick
);
console.log(
  'See that the task still exists:',
  (button as any).__zone_symbol__clickfalse
);

Please provide the environment you discovered this bug in (run ng version)

edit: affects Angular 17 too

(running on Stackblitz, reproducible locally too, even without hot reloading)

@angular/common
16.2.12

@angular/compiler
16.2.12

@angular/core
16.2.12

@angular/platform-browser
16.2.12

@types/jasmine
4.3.6

rxjs
7.8.1

tslib
2.6.2

zone.js
0.13.3

Anything else?

I know that using on* is bad-practice, but it's coming from a library I rely on (for now). The capture:true events being set and removed is also from a library I rely on, so I can't easily remove these preconditions for the bug. As a workaround, I will use element ref to manually add/removeEventListener, but I am sure that there would be other places in my application affected by this bug

cc @JiaLiPassion

[andygup]

We've also verified this issue exists on version 17.2.2.

[JiaLiPassion]

@maxpatiiuk
thank you for reporting the issue and the detail reproduce details, I created a PR to fix this one.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}