fix: don't delegate events on custom elements, solve edge case stopPropagation issue

- don't delegate events on custom elements
- still invoke listener for cancelled event on the element where it was cancelled: when you do `stopPropagation`, `event.cancelBubble` becomes `true`. We can't use this as an indicator to not invoke a listener directly, because the listner could be on the element where propagation was cancelled, i.e. it should still run for that listener. Instead, adjust the event propagation algorithm to detect when a delegated event listener caused the event to be cancelled

fixes #14704
event-delegation-fixes
Simon Holthausen 8 months ago
parent c4e9faad52
commit c05abb5e54

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: don't delegate events on custom elements

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: still invoke listener for cancelled event on the element where it was cancelled

@ -7,6 +7,7 @@ import {
get_attribute_expression,
is_event_attribute
} from '../../../utils/ast.js';
import { is_custom_element_node } from '../../nodes.js';
import { mark_subtree_dynamic } from './shared/fragment.js';
/**
@ -67,7 +68,6 @@ export function Attribute(node, context) {
}
if (is_event_attribute(node)) {
const parent = context.path.at(-1);
if (parent?.type === 'RegularElement' || parent?.type === 'SvelteElement') {
context.state.analysis.uses_event_attributes = true;
}
@ -75,7 +75,14 @@ export function Attribute(node, context) {
const expression = get_attribute_expression(node);
const delegated_event = get_delegated_event(node.name.slice(2), expression, context);
if (delegated_event !== null) {
if (
delegated_event !== null &&
// We can't assume that the events from within the shadow root bubble beyond it.
// If someone dispatches them without the composed option, they won't. Also
// people could repurpose the event names to do something else, or call stopPropagation
// on the shadow root so it doesn't bubble beyond it.
!(parent?.type === 'RegularElement' && is_custom_element_node(parent))
) {
if (delegated_event.hoisted) {
delegated_event.function.metadata.hoisted = true;
}

@ -318,7 +318,8 @@ export function set_attributes(
const opts = {};
const event_handle_key = '$$' + key;
let event_name = key.slice(2);
var delegated = is_delegated(event_name);
// Events on custom elements can be anything, we can't assume they bubble
var delegated = !is_custom_element && is_delegated(event_name);
if (is_capture_event(event_name)) {
event_name = event_name.slice(0, -7);

@ -61,7 +61,10 @@ export function create_event(event_name, dom, handler, options) {
// Only call in the bubble phase, else delegated events would be called before the capturing events
handle_event_propagation.call(dom, event);
}
if (!event.cancelBubble) {
// @ts-expect-error Use this instead of cancelBubble, because cancelBubble is also true if
// we're the last element on which the event will be handled.
if (!event.__cancelled || event.__cancelled === dom) {
return without_reactive_context(() => {
return handler.call(this, event);
});
@ -171,6 +174,8 @@ export function handle_event_propagation(event) {
// chain in case someone manually dispatches the same event object again.
// @ts-expect-error
event.__root = handler_element;
// @ts-expect-error
event.__cancelled = null;
return;
}
@ -216,6 +221,7 @@ export function handle_event_propagation(event) {
set_active_effect(null);
try {
var cancelled = event.cancelBubble;
/**
* @type {unknown}
*/
@ -253,6 +259,10 @@ export function handle_event_propagation(event) {
}
}
if (event.cancelBubble || parent_element === handler_element || parent_element === null) {
if (!cancelled && event.cancelBubble) {
// @ts-expect-error
event.__cancelled = current_target;
}
break;
}
current_target = parent_element;

@ -0,0 +1,18 @@
import { test } from '../../test';
export default test({
mode: ['client'],
async test({ assert, target, logs }) {
const [btn1, btn2] = [...target.querySelectorAll('custom-element')].map((c) =>
c.shadowRoot?.querySelector('button')
);
btn1?.click();
await Promise.resolve();
assert.deepEqual(logs, ['reached shadow root1']);
btn2?.click();
await Promise.resolve();
assert.deepEqual(logs, ['reached shadow root1', 'reached shadow root2']);
}
});

@ -0,0 +1,19 @@
<script>
class CustomElement extends HTMLElement {
constructor() {
super();
this.attachShadow({ mode: 'open' });
this.shadowRoot.innerHTML = '<button>click me</button>';
// Looks weird, but some custom element implementations actually do this
// to prevent unwanted side upwards event propagation
this.addEventListener('click', (e) => e.stopPropagation());
}
}
customElements.define('custom-element', CustomElement);
</script>
<div onclick={() => console.log('bubbled beyond shadow root')}>
<custom-element onclick={() => console.log('reached shadow root1')}></custom-element>
<custom-element {...{onclick:() => console.log('reached shadow root2')}}></custom-element>
</div>
Loading…
Cancel
Save