Skip to content

Commit bd016bb

Browse files
Use a helper function for all DOM id gets to avoid issues with HTMLFormElement.id being untrustworthy.
For more details: * #135 * https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement#issues_with_naming_elements
1 parent 7cea752 commit bd016bb

File tree

1 file changed

+21
-14
lines changed

1 file changed

+21
-14
lines changed

src/idiomorph.js

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ var Idiomorph = (function () {
225225

226226
const results = fn();
227227

228-
if (activeElementId && activeElementId !== document.activeElement?.id) {
228+
if (activeElementId && activeElementId !== id(document.activeElement)) {
229229
activeElement = ctx.target.querySelector(`[id="${activeElementId}"]`);
230230
activeElement?.focus();
231231
}
@@ -304,11 +304,11 @@ var Idiomorph = (function () {
304304
}
305305

306306
// if the matching node is elsewhere in the original content
307-
if (newChild instanceof Element && ctx.persistentIds.has(newChild.id)) {
307+
if (newChild instanceof Element && ctx.persistentIds.has(id(newChild))) {
308308
// move it and all its children here and morph
309309
const movedChild = moveBeforeById(
310310
oldParent,
311-
newChild.id,
311+
id(newChild),
312312
insertionPoint,
313313
ctx,
314314
);
@@ -474,7 +474,7 @@ var Idiomorph = (function () {
474474
// If oldElt has an `id` with possible state and it doesn't match newElt.id then avoid morphing.
475475
// We'll still match an anonymous node with an IDed newElt, though, because if it got this far,
476476
// its not persistent, and new nodes can't have any hidden state.
477-
(!oldElt.id || oldElt.id === newElt.id)
477+
(!id(oldElt) || id(oldElt) === id(newElt))
478478
);
479479
}
480480

@@ -528,19 +528,19 @@ var Idiomorph = (function () {
528528
* Search for an element by id within the document and pantry, and move it using moveBefore.
529529
*
530530
* @param {Element} parentNode - The parent node to which the element will be moved.
531-
* @param {string} id - The ID of the element to be moved.
531+
* @param {string} elementId - The ID of the element to be moved.
532532
* @param {Node | null} after - The reference node to insert the element before.
533533
* If `null`, the element is appended as the last child.
534534
* @param {MorphContext} ctx
535535
* @returns {Element} The found element
536536
*/
537-
function moveBeforeById(parentNode, id, after, ctx) {
537+
function moveBeforeById(parentNode, elementId, after, ctx) {
538538
const target =
539539
/** @type {Element} - will always be found */
540540
(
541-
(ctx.target.id === id && ctx.target) ||
542-
ctx.target.querySelector(`[id="${id}"]`) ||
543-
ctx.pantry.querySelector(`[id="${id}"]`)
541+
(id(ctx.target) === elementId && ctx.target) ||
542+
ctx.target.querySelector(`[id="${elementId}"]`) ||
543+
ctx.pantry.querySelector(`[id="${elementId}"]`)
544544
);
545545
removeElementFromAncestorsIdMaps(target, ctx);
546546
moveBefore(parentNode, target, after);
@@ -556,12 +556,12 @@ var Idiomorph = (function () {
556556
* @param {MorphContext} ctx
557557
*/
558558
function removeElementFromAncestorsIdMaps(element, ctx) {
559-
const id = element.id;
559+
const elementId = id(element);
560560
/** @ts-ignore - safe to loop in this way **/
561561
while ((element = element.parentNode)) {
562562
let idSet = ctx.idMap.get(element);
563563
if (idSet) {
564-
idSet.delete(id);
564+
idSet.delete(elementId);
565565
if (!idSet.size) {
566566
ctx.idMap.delete(element);
567567
}
@@ -1044,7 +1044,7 @@ var Idiomorph = (function () {
10441044
*/
10451045
function findIdElements(root) {
10461046
let elements = Array.from(root.querySelectorAll("[id]"));
1047-
if (root.id) {
1047+
if (id(root)) {
10481048
elements.push(root);
10491049
}
10501050
return elements;
@@ -1063,7 +1063,7 @@ var Idiomorph = (function () {
10631063
*/
10641064
function populateIdMapWithTree(idMap, persistentIds, root, elements) {
10651065
for (const elt of elements) {
1066-
if (persistentIds.has(elt.id)) {
1066+
if (persistentIds.has(id(elt))) {
10671067
/** @type {Element|null} */
10681068
let current = elt;
10691069
// walk up the parent hierarchy of that element, adding the id
@@ -1075,7 +1075,7 @@ var Idiomorph = (function () {
10751075
idSet = new Set();
10761076
idMap.set(current, idSet);
10771077
}
1078-
idSet.add(elt.id);
1078+
idSet.add(id(elt));
10791079

10801080
if (current === root) break;
10811081
current = current.parentElement;
@@ -1335,6 +1335,13 @@ var Idiomorph = (function () {
13351335
return { normalizeElement, normalizeParent };
13361336
})();
13371337

1338+
function id(node) {
1339+
// node could be a <form> so we can't trust node.id:
1340+
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement#issues_with_naming_elements
1341+
// node could be a document-fragment which doesn't have getAttribute defined
1342+
return node.getAttribute && node.getAttribute("id");
1343+
}
1344+
13381345
//=============================================================================
13391346
// This is what ends up becoming the Idiomorph global object
13401347
//=============================================================================

0 commit comments

Comments
 (0)