From 87706792cb03077c7e55ca4a94e44ad2f2cfe858 Mon Sep 17 00:00:00 2001
From: Sam Sung <51686767+samwhosung@users.noreply.github.com>
Date: Fri, 9 Oct 2026 11:18:42 +0700
Subject: [PATCH] A frame or region takes no name or id from its template
 (#839)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

An instance with no `name=` took its template's name: `framexml::merge` laid the instance's attributes over the template's, so `name` and `id` carried through. Stock `UIOptionsFrame.xml`'s unnamed `<Button inherits="UIPanelCloseButton">` became the global `UIPanelCloseButton`, with `GetName()` answering it, and any unnamed addon frame did the same to its template's name.

In 1.12.1, `PreLoadXML` (`0x769770`) reads `name` and `id` from the instance node before `LoadXML` walks the templates (`0x76985c`). The texture and font string factories also read `name` once, outside that walk (`0x6f2751`, `0x6f27e1`). So neither is ever inherited.

The merge now drops the template's `name` and `id`. Tests cover an unnamed frame, a named frame (no template id), an unnamed texture and `CreateFrame` with a nil name. All four fail without the fix.

Fixes #394

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 crates/benilla-ui/src/framexml.rs             | 16 +++-
 crates/benilla-ui/src/loader/mod.rs           |  2 +
 .../src/loader/template_name_tests.rs         | 87 +++++++++++++++++++
 3 files changed, 101 insertions(+), 4 deletions(-)
 create mode 100644 crates/benilla-ui/src/loader/template_name_tests.rs

diff --git a/crates/benilla-ui/src/framexml.rs b/crates/benilla-ui/src/framexml.rs
index 0e1cdfe1..0296fd23 100644
--- a/crates/benilla-ui/src/framexml.rs
+++ b/crates/benilla-ui/src/framexml.rs
@@ -315,8 +315,10 @@ fn element_from_node(node: roxmltree::Node) -> Element {
 /// attributes override or extend the template's (case-insensitively). The reference merges
 /// nothing: `LoadXML` re-enters itself on the one object against each template, the most distant
 /// first, then the instance (`0x76985c`; `<Frames>` repeat the walk, `0x76a060`), and reads `name`
-/// and `id` from the instance alone (`PreLoadXML 0x769770`). Here `name` and `virtual` merge like
-/// any attribute. A cycle is skipped with a warning.
+/// and `id` from the instance alone (`PreLoadXML 0x769770`; a region's factory reads its `name`
+/// once, outside the walk, `0x6f2751`/`0x6f27e1`). So here `name` and `id` come from the element
+/// alone, and an unnamed instance stays unnamed; `virtual` merges like any attribute, read only at
+/// registration. A cycle is skipped with a warning.
 pub fn expand(
     element: &Element,
     templates: &HashMap<&str, &Element>,
@@ -390,9 +392,15 @@ fn expand_inner(
     }
 }
 
-/// [`expand`]'s merge; `over`'s tag wins, and its body when it has one.
+/// [`expand`]'s merge; `over`'s tag wins, and its body when it has one. `base`'s `name` and `id`
+/// are dropped: they belong to the node they are written on.
 fn merge(base: &Element, over: &Element) -> Element {
-    let mut attrs = base.attrs.clone();
+    let mut attrs: Vec<(String, String)> = base
+        .attrs
+        .iter()
+        .filter(|(k, _)| !k.eq_ignore_ascii_case("name") && !k.eq_ignore_ascii_case("id"))
+        .cloned()
+        .collect();
     for (k, v) in &over.attrs {
         if let Some(slot) = attrs.iter_mut().find(|(ek, _)| ek.eq_ignore_ascii_case(k)) {
             slot.1 = v.clone();
diff --git a/crates/benilla-ui/src/loader/mod.rs b/crates/benilla-ui/src/loader/mod.rs
index 1084f84f..0e6d9d6c 100644
--- a/crates/benilla-ui/src/loader/mod.rs
+++ b/crates/benilla-ui/src/loader/mod.rs
@@ -898,5 +898,7 @@ pub(super) fn font_object_from_element(el: &Element) -> FontObject {
     }
 }
 
+#[cfg(test)]
+mod template_name_tests;
 #[cfg(test)]
 mod tests;
diff --git a/crates/benilla-ui/src/loader/template_name_tests.rs b/crates/benilla-ui/src/loader/template_name_tests.rs
new file mode 100644
index 00000000..f0e12d21
--- /dev/null
+++ b/crates/benilla-ui/src/loader/template_name_tests.rs
@@ -0,0 +1,87 @@
+//! `name=` and `id=` belong to the node they are written on: `PreLoadXML 0x769770` reads them from
+//! the instance before `LoadXML` walks its templates (`0x76985c`), and the region factories read
+//! `name` once, outside that walk (`0x6f2751`, `0x6f27e1`).
+
+use crate::framexml;
+use crate::loader::load;
+use crate::script::UiScript;
+
+const DOC: &str = r#"<Ui>
+    <Button name="TmpCloseTemplate" id="3" virtual="true">
+        <Size><AbsDimension x="32" y="32"/></Size>
+    </Button>
+    <Texture name="TmpTexTemplate" virtual="true">
+        <Color r="1" g="0" b="0" a="1"/>
+    </Texture>
+    <Frame name="Holder">
+        <Size><AbsDimension x="64" y="64"/></Size>
+        <Anchors><Anchor point="CENTER"/></Anchors>
+        <Layers>
+            <Layer level="ARTWORK">
+                <Texture inherits="TmpTexTemplate"/>
+            </Layer>
+        </Layers>
+        <Frames>
+            <Button inherits="TmpCloseTemplate"/>
+            <Button name="$parentClose" inherits="TmpCloseTemplate"/>
+        </Frames>
+    </Frame>
+</Ui>"#;
+
+fn loaded() -> UiScript {
+    let mut s = UiScript::new().unwrap();
+    s.set_screen_size(1024.0, 768.0);
+    let report = load(&s, &framexml::parse(DOC).unwrap(), &|_| None);
+    assert!(report.errors.is_empty(), "{:?}", report.errors);
+    s
+}
+
+/// Stock `UIOptionsFrame.xml:1477`'s unnamed `<Button inherits="UIPanelCloseButton">` is the
+/// shape: in 1.12.1 the template's name is no global and the button answers no name.
+#[test]
+fn an_unnamed_instance_takes_neither_its_templates_name_nor_its_id() {
+    let s = loaded();
+    assert!(
+        s.eval::<bool>("return TmpCloseTemplate == nil").unwrap(),
+        "the unnamed instance published the template's name as a global"
+    );
+    assert!(s
+        .eval::<bool>("return ({Holder:GetChildren()})[1]:GetName() == nil")
+        .unwrap());
+    assert_eq!(
+        s.eval::<i64>("return ({Holder:GetChildren()})[1]:GetID()")
+            .unwrap(),
+        0
+    );
+}
+
+#[test]
+fn a_named_instance_keeps_its_own_name_and_takes_no_template_id() {
+    let s = loaded();
+    assert_eq!(
+        s.eval::<String>("return HolderClose:GetName()").unwrap(),
+        "HolderClose"
+    );
+    assert_eq!(s.eval::<i64>("return HolderClose:GetID()").unwrap(), 0);
+}
+
+#[test]
+fn an_unnamed_region_takes_no_name_from_its_template() {
+    let s = loaded();
+    assert!(s.eval::<bool>("return TmpTexTemplate == nil").unwrap());
+    assert!(s
+        .eval::<bool>("return ({Holder:GetRegions()})[1]:GetName() == nil")
+        .unwrap());
+}
+
+/// `CreateFrame` builds its node with the `name` it was given (`0x70622d`), nil included.
+#[test]
+fn create_frame_with_no_name_stays_unnamed_under_a_named_template() {
+    let s = loaded();
+    assert!(s
+        .eval::<bool>(
+            "local b = CreateFrame('Button', nil, Holder, 'TmpCloseTemplate') \
+             return b:GetName() == nil and TmpCloseTemplate == nil"
+        )
+        .unwrap());
+}