Skip to content

Commit

Permalink
fix: don't delegate events on custom elements, solve edge case stopPr…
Browse files Browse the repository at this point in the history
…opagation 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
  • Loading branch information
dummdidumm committed Jan 6, 2025
1 parent c4e9faa commit c05abb5
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/kind-wombats-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: don't delegate events on custom elements
5 changes: 5 additions & 0 deletions .changeset/six-steaks-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: still invoke listener for cancelled event on the element where it was cancelled
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -67,15 +68,21 @@ 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;
}

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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 11 additions & 1 deletion packages/svelte/src/internal/client/dom/elements/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -216,6 +221,7 @@ export function handle_event_propagation(event) {
set_active_effect(null);

try {
var cancelled = event.cancelBubble;
/**
* @type {unknown}
*/
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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']);
}
});
Original file line number Diff line number Diff line change
@@ -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>

0 comments on commit c05abb5

Please sign in to comment.