Skip to content

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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>
Loading