1915650 - The new loading.svg markup could lead to duplicate IDs in the DOM

Status: RESOLVED INVALID

(Toolkit :: Themes, defect)

Description

[Erik Nordin [:nordzilla]]

Description

Bug 1908920 introduced new semantics to loading.svg that allow it to switch between the spinning circle-arrows image or the static hourglass image based on the user's prefers-reduced-motion setting.

However, the initial implementation uses id attributes to determine the animation state. This could potentially lead to duplicate IDs in the DOM if two elements include that same SVG.

We should probably use class here instead of id. The functionality should be equivalent, and more correct in the situation of multiple elements displaying this SVG.

Comment 1

[Erik Nordin [:nordzilla]]

Attachment: Bug 1915650 - Use class instead of id in loading.svg r=dao!

The new loading.svg semantics use id attributes to
determine which image to display based on the user's
prefers-reduced-motion settings. It would be better
to use class here in case two elements display the
same SVG within the same DOM tree.

Comment 2

[Erik Nordin [:nordzilla]]

It turns out that the DOM in the image is not part of the DOM of the document that includes the image. This means that there isn't actually a risk of duplicate ID's.

! In D220577#7577988, @Gijs wrote:

The DOM in the image is not part of the DOM of the document that includes the image (when loading via <img> or CSS background-image/list-style-image), so there can't be any conflicts as a result of using id.

The only case where this would be a problem is if we used #include or similar to copy the markup into multiple places. But we don't do that for images - apart from this issue (and there are a bunch of other SVGs with ids) it would also result in scripts running and would break imglib caching things. So even though using classes doesn't hurt much, I don't think we need to take this as such.