Skip to content

Commit d3dd291

Browse files
committed
fix: robust handling of events in spread attributes
When an event listener is removed right before it would be fired, adding a new listener comes too late for the browser and the event is swallowed. The fix is to tweak the spread attributes function to keep using the same object for updates so that we can wrap the event listener to invoke it like `attributes[key](evt)` instead. No test because it seems impossible to reproduce in testing environments. Fixes #11903
1 parent 7714c93 commit d3dd291

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

.changeset/cyan-toes-share.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: robust handling of events in spread attributes

packages/svelte/src/internal/client/dom/elements/attributes.js

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ export function set_custom_element_data(node, prop, value) {
144144
*/
145145
export function set_attributes(element, prev, next, lowercase_attributes, css_hash) {
146146
var has_hash = css_hash.length !== 0;
147+
var current = prev || {};
147148

148149
for (var key in prev) {
149150
if (!(key in next)) {
@@ -167,7 +168,10 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
167168
for (const key in next) {
168169
// let instead of var because referenced in a closure
169170
let value = next[key];
170-
if (value === prev?.[key]) continue;
171+
var prev_value = current[key];
172+
if (value === prev_value) continue;
173+
174+
current[key] = value;
171175

172176
var prefix = key[0] + key[1]; // this is faster than key.slice(0, 2)
173177
if (prefix === '$$') continue;
@@ -183,21 +187,35 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
183187
opts.capture = true;
184188
}
185189

186-
if (!delegated && prev?.[key]) {
187-
element.removeEventListener(event_name, /** @type {any} */ (prev[key]), opts);
190+
if (!delegated && prev_value) {
191+
// Listening to same event but different handler -> our handle function below takes care of this
192+
// If we were to remove and add listeners in this case, it could happen that the event is "swallowed"
193+
// (the browser seems to not know yet that a new one exists now) and doesn't reach the handler
194+
// https://github.com/sveltejs/svelte/issues/11903
195+
if (value != null) continue;
196+
197+
element.removeEventListener(event_name, current['$$' + key], opts);
198+
current['$$' + key] = null;
188199
}
189200

190201
if (value != null) {
191202
if (!delegated) {
192-
// we use `addEventListener` here because these events are not delegated
203+
/**
204+
* @this {any}
205+
* @param {Event} evt
206+
*/
207+
function handle(evt) {
208+
current[key].call(this, evt);
209+
}
210+
193211
if (!prev) {
194212
events.push([
195213
key,
196214
value,
197-
() => (next[key] = create_event(event_name, element, value, opts))
215+
() => (current['$$' + key] = create_event(event_name, element, handle, opts))
198216
]);
199217
} else {
200-
next[key] = create_event(event_name, element, value, opts);
218+
current['$$' + key] = create_event(event_name, element, handle, opts);
201219
}
202220
} else {
203221
// @ts-ignore
@@ -252,7 +270,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
252270
effect(() => {
253271
if (!element.isConnected) return;
254272
for (const [key, value, evt] of events) {
255-
if (next[key] === value) {
273+
if (current[key] === value) {
256274
evt();
257275
}
258276
}
@@ -261,7 +279,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
261279
});
262280
}
263281

264-
return next;
282+
return current;
265283
}
266284

267285
/**

0 commit comments

Comments
 (0)