Backport fixes from aframe-inspector (batch 5)

[vincentfretin]

Backport fixes from aframe-inspector (batch 5)

Previous sync: #1180

Changes

• Allow dash in component instance id like bind__troika-text — aframevr/aframe-inspector#822
• Deduplicate asset registration, create a-assets if missing, show error for duplicate/invalid names — aframevr/aframe-inspector#824
• Fix CSS color widget pseudo-element selectors (missing :: prefix) — aframevr/aframe-inspector#826
• Remove useless THREE.ImageUtils.crossOrigin line — aframevr/aframe-inspector#832
• Fix clicking on boolean label, remove unused widget propTypes, add id to all widgets — aframevr/aframe-inspector#819
• Disable spellcheck on input fields — aframevr/aframe-inspector#825
• Fix entity reparent to preserve world scale — c-frame/aframe-editor#62
• Remove default rotation/scale attributes after reparent — c-frame/aframe-editor#63
• Remove non-existing animatetop animation from texture modal — aframevr/aframe-inspector#837

[claude]

Code Review — Backport fixes from aframe-inspector (batch 5)

Great batch of upstream fixes. The changes are well-scoped, clearly motivated, and generally clean. Here's a detailed look:

---

Overview

This PR backports 9 upstream fixes from aframe-inspector, covering: component ID validation, asset deduplication/error handling, CSS pseudo-element bugs, prop interface cleanup, boolean label click fix, world-scale preservation on reparent, and UX polish.

---

What's Well Done

• CSS pseudo-element fix (index.scss) — The missing :: prefix on -webkit-color-swatch etc. was a legitimate rendering bug. Clean fix.
• assetsUtils.js consolidation — Moving shared helpers out of ModalTextures and TextureWidget into a single module eliminates duplication and makes them testable in isolation.
• a-assets auto-creation in insertNewAsset — Good defensive addition. Throwing a clear error when a-scene is also missing is better than a silent crash on null.appendChild.
• crossOrigin = 'anonymous' always-on — Removing the conditional is safe here; every call site was passing true anyway, and the TextureWidget's private copy always set it unconditionally.
• ID consistency fix for BooleanWidget — The root cause of the label-click bug was a separator mismatch: PropertyRow.render() used htmlFor={componentname + ':' + name} while BooleanWidget independently generated componentname + '.' + name. Centralizing the id in PropertyRow's constructor (this.id = props.componentname + ':' + props.name) and threading it through widgetProps fixes this cleanly.
• isMap simplification in PropertyRow — Confirmed safe: A-Frame's material.src schema already has type: 'map'; only envMap has the wrong type string. The comment makes this clear.
• Scale preservation in EntityReparentCommand — Correctly captures world scale before reparent and recomputes local scale relative to the new parent's world scale. The removeAttribute optimization for identity transforms is a nice touch.

---

Issues

1. Floating-point equality after trig operations — EntityReparentCommand.js:88-99

if (rotX === 0 && rotY === 0 && rotZ === 0) {
  entity.removeAttribute('rotation');
}
if (newLocalScale.x === 1 && newLocalScale.y === 1 && newLocalScale.z === 1) {
  entity.removeAttribute('scale');
}

After setFromQuaternion → radToDeg and after world-scale division, these values will almost never be exactly 0 or 1 due to floating-point drift. An epsilon comparison (e.g. Math.abs(rotX) < 1e-5) would be more reliable. In practice this means attributes will often be written with near-zero/near-one values instead of being omitted — a minor inefficiency rather than a hard bug.

(This appears to be carried over from the upstream PRs, so not a blocker for this backport.)

2. Regex missing g flag in getValidId — assetsUtils.js:29-35

.replace(/\s/, '-')      // only replaces the FIRST space
.replace(/[\W]/, '')     // only replaces the FIRST non-word char

These were copied verbatim from the removed ModalTextures.js code, so this is a pre-existing bug, not introduced here. But since the function is now centralized and exported it's more visible. Consider fixing:

.replace(/\s+/g, '-')
.replace(/\W+/g, '')

3. CSS selector injection risk — assetsUtils.js:10

export function getIdFromUrl(url) {
  return document.querySelector("a-assets > [src='" + url + "']")?.id;
}

If url contains a single quote (e.g. a malformed or adversarial URL), this will break the querySelector call with a SyntaxError. URLs sourced from user-supplied input should be escaped. At minimum:

const escaped = url.replace(/'/g, "\\'");
return document.querySelector(`a-assets > [src='${escaped}']`)?.id;

The same issue exists in the pre-existing getUrlFromId which passes assetId directly to querySelector. Low severity since these are editor-only surfaces, but worth noting.

4. TextureWidget auto-registration removal — TextureWidget.js:140

The old onTextureModalSet called insertOrGetImageAsset for non-asset typed images, which would auto-register the image in a-assets and return a #id reference. This is now removed entirely. The new assumption is that ModalTextures.addNewAsset has already registered the asset before the event fires. Confirm that:

• Selecting a registry image that is NOT yet in a-assets still results in it being registered before the texture value is set on the entity.
• The event payload's value field for newly-added assets is #assetId (not url(...)) — looking at preview.value = 'url(' + src + ')' in ModalTextures, this may need verification.

5. onNameKeyUp inline handler recreated on each render — ModalTextures.js

Minor: the extracted inline onKeyUp arrow function closes over validAsset:

onKeyUp={(event) => {
  if (event.keyCode === 13 && validAsset) {
    this.addNewAsset();
  }
}}

This is fine functionally (the closure captures the correct value), but note event.keyCode is deprecated — event.key === 'Enter' is the modern equivalent.

---

Test Coverage

No new tests accompany this PR. The fixes are largely UI/behavioral (clicking labels, CORS attributes, CSS rendering, reparent math), so unit tests are harder to write — but EntityReparentCommand.updateLocalTransform with varying parent scales would be a good candidate for a Jest test, especially the floating-point equality edge case.

---

Summary

Good backport — the fixes address real bugs and the code is cleaner post-PR. The main things to double-check before merging:

1. Manual test: drag-reparent an entity under a scaled parent and verify world position, rotation, and scale are preserved.
2. Verify the TextureWidget/ModalTextures event flow for newly-added (non-asset) textures still works end-to-end.
3. Optional: fix the g-flag regex in getValidId and the CSS selector injection in getIdFromUrl.

[kfarr]

thanks @vincentfretin looks good. was able to test a few fixes but not everything since we don't support reparenting yet

[vincentfretin]

summary point 3 addressed in #1558