Skip to content

Commit cd2071e

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 cd2071e

File tree

1 file changed

+25
-14
lines changed

1 file changed

+25
-14
lines changed

src/idiomorph.js

Lines changed: 25 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,14 @@ 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 (
308+
newChild instanceof Element &&
309+
ctx.persistentIds.has(id(newChild))
310+
) {
308311
// move it and all its children here and morph
309312
const movedChild = moveBeforeById(
310313
oldParent,
311-
newChild.id,
314+
id(newChild),
312315
insertionPoint,
313316
ctx,
314317
);
@@ -474,7 +477,7 @@ var Idiomorph = (function () {
474477
// If oldElt has an `id` with possible state and it doesn't match newElt.id then avoid morphing.
475478
// We'll still match an anonymous node with an IDed newElt, though, because if it got this far,
476479
// its not persistent, and new nodes can't have any hidden state.
477-
(!oldElt.id || oldElt.id === newElt.id)
480+
(!id(oldElt) || id(oldElt) === id(newElt))
478481
);
479482
}
480483

@@ -528,19 +531,19 @@ var Idiomorph = (function () {
528531
* Search for an element by id within the document and pantry, and move it using moveBefore.
529532
*
530533
* @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.
534+
* @param {string} elementId - The ID of the element to be moved.
532535
* @param {Node | null} after - The reference node to insert the element before.
533536
* If `null`, the element is appended as the last child.
534537
* @param {MorphContext} ctx
535538
* @returns {Element} The found element
536539
*/
537-
function moveBeforeById(parentNode, id, after, ctx) {
540+
function moveBeforeById(parentNode, elementId, after, ctx) {
538541
const target =
539542
/** @type {Element} - will always be found */
540543
(
541-
(ctx.target.id === id && ctx.target) ||
542-
ctx.target.querySelector(`[id="${id}"]`) ||
543-
ctx.pantry.querySelector(`[id="${id}"]`)
544+
(id(ctx.target) === elementId && ctx.target) ||
545+
ctx.target.querySelector(`[id="${elementId}"]`) ||
546+
ctx.pantry.querySelector(`[id="${elementId}"]`)
544547
);
545548
removeElementFromAncestorsIdMaps(target, ctx);
546549
moveBefore(parentNode, target, after);
@@ -556,12 +559,12 @@ var Idiomorph = (function () {
556559
* @param {MorphContext} ctx
557560
*/
558561
function removeElementFromAncestorsIdMaps(element, ctx) {
559-
const id = element.id;
562+
const elementId = id(element);
560563
/** @ts-ignore - safe to loop in this way **/
561564
while ((element = element.parentNode)) {
562565
let idSet = ctx.idMap.get(element);
563566
if (idSet) {
564-
idSet.delete(id);
567+
idSet.delete(elementId);
565568
if (!idSet.size) {
566569
ctx.idMap.delete(element);
567570
}
@@ -1044,7 +1047,7 @@ var Idiomorph = (function () {
10441047
*/
10451048
function findIdElements(root) {
10461049
let elements = Array.from(root.querySelectorAll("[id]"));
1047-
if (root.id) {
1050+
if (id(root)) {
10481051
elements.push(root);
10491052
}
10501053
return elements;
@@ -1063,7 +1066,7 @@ var Idiomorph = (function () {
10631066
*/
10641067
function populateIdMapWithTree(idMap, persistentIds, root, elements) {
10651068
for (const elt of elements) {
1066-
if (persistentIds.has(elt.id)) {
1069+
if (persistentIds.has(id(elt))) {
10671070
/** @type {Element|null} */
10681071
let current = elt;
10691072
// walk up the parent hierarchy of that element, adding the id
@@ -1075,7 +1078,7 @@ var Idiomorph = (function () {
10751078
idSet = new Set();
10761079
idMap.set(current, idSet);
10771080
}
1078-
idSet.add(elt.id);
1081+
idSet.add(id(elt));
10791082

10801083
if (current === root) break;
10811084
current = current.parentElement;
@@ -1335,6 +1338,14 @@ var Idiomorph = (function () {
13351338
return { normalizeElement, normalizeParent };
13361339
})();
13371340

1341+
// @ts-ignore Nodeish is fine
1342+
function id(node) {
1343+
// node could be a <form> so we can't trust node.id:
1344+
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement#issues_with_naming_elements
1345+
// node could be a document-fragment which doesn't have getAttribute defined
1346+
return node.getAttribute && node.getAttribute("id");
1347+
}
1348+
13381349
//=============================================================================
13391350
// This is what ends up becoming the Idiomorph global object
13401351
//=============================================================================

0 commit comments

Comments
 (0)