From 010d88024ce98d2a1789e1dd9f5039849c844826 Mon Sep 17 00:00:00 2001 From: Kai Date: Wed, 22 Jul 2026 14:20:53 -0700 Subject: [PATCH 1/2] TEMPLATE-570: preserve attributes on repeat unbind --- .../loop-attrs/repeat-attr/index.js | 10 ++- .../repeat-attr/tests/test-repeat-attr.js | 82 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js b/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js index ec0490a8..4d40554e 100644 --- a/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js +++ b/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js @@ -34,7 +34,15 @@ const loopAttrHandler = { const originalHTML = getOriginalContents($el); const current = $el.get(0).outerHTML; if (originalHTML && current !== originalHTML) { - $el.replaceWith(originalHTML); + // TEMPLATE-570: Restore the element to its pristine template form by resetting only its inner + // content, keeping the element itself and its current attributes. We intentionally + // do NOT replace the whole element with the captured snapshot: the snapshot is taken + // on first render, so replacing would revert any later edit to the element's own + // attributes (e.g. changing data-f-repeat in the interface builder) and detach the + // live node, discarding the change. + const templateInnerHTML = $(originalHTML).html(); + $el.html(templateInnerHTML); + $el.removeAttr('hidden'); } clearOriginalContents($el); removeKnownData($el); diff --git a/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/tests/test-repeat-attr.js b/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/tests/test-repeat-attr.js index 910f0477..da08bce9 100644 --- a/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/tests/test-repeat-attr.js +++ b/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/tests/test-repeat-attr.js @@ -277,5 +277,87 @@ describe('Repeat', function () { repeatHandler.unbind('repeat', $rootNode.find('li:first')); $rootNode.children().length.should.equal(3); }); + + it('should preserve edits to the element`s own attributes instead of reverting them (TEMPLATE-570)', ()=> { + var $rootNode = $(''); + const topics = [{ name: 'oldVar' }]; + const $el = $rootNode.find('li:first'); + const originalNode = $el.get(0); + + // First render captures the template snapshot and generates sibling clones. + repeatHandler.handle([1, 2, 3], 'repeat', $el, topics); + $rootNode.children().length.should.equal(3); + + // Simulate an interface-builder edit of the bound variable. + $el.attr('data-f-repeat', 'newVar'); + + repeatHandler.unbind('repeat', $el); + + // Generated siblings are cleaned up, leaving only the template element... + $rootNode.children().length.should.equal(1); + // ...the same node stays in the DOM (not detached/replaced)... + $rootNode.children().get(0).should.equal(originalNode); + // ...and the edited attribute survives rather than reverting to "oldVar". + // NOTE: read the node actually left in the DOM, not the $el handle. Under the old + // buggy replaceWith the $el handle pointed at the *detached* node (which still carried + // "newVar"), so asserting on $el would pass even against the bug. + $($rootNode.children().get(0)).attr('data-f-repeat').should.equal('newVar'); + // ...and the inner content is restored to the template, not left as rendered data. + // Use .text(): serializing a text node via .html() would HTML-escape the < and >. + $($rootNode.children().get(0)).text().trim().should.equal('<%= value %>'); + }); + + it('should preserve attribute edits on table elements (data-f-repeat on ) (TEMPLATE-570)', ()=> { + var $rootNode = $('
<%= value %>
'); + const topics = [{ name: 'oldVar' }]; + const $el = $rootNode.find('td:first'); + const originalNode = $el.get(0); + + repeatHandler.handle([1, 2, 3], 'repeat', $el, topics); + // The template td plus its two generated sibling clones. + $rootNode.find('tr:first').children().length.should.equal(3); + + $el.attr('data-f-repeat', 'newVar'); + repeatHandler.unbind('repeat', $el); + + const $cells = $rootNode.find('tr:first').children(); + // Siblings cleaned up, same node kept, edit preserved, inner template restored. + $cells.length.should.equal(1); + $cells.get(0).should.equal(originalNode); + $($cells.get(0)).attr('data-f-repeat').should.equal('newVar'); + $($cells.get(0)).text().trim().should.equal('<%= value %>'); + }); + + it('should stay idempotent across repeated handle/unbind cycles (no row growth)', ()=> { + var $rootNode = $(''); + const topics = [{ name: 'somearray' }]; + const $el = $rootNode.find('li:first'); + const data = [1, 2, 3]; + + for (var i = 0; i < 3; i++) { + repeatHandler.handle(data, 'repeat', $el, topics); + // One template element + one clone per data item. + $rootNode.children().length.should.equal(data.length); + + repeatHandler.unbind('repeat', $el); + // Back down to just the template element; clones fully cleaned up. + $rootNode.children().length.should.equal(1); + } + }); + + it('should remove the hidden attribute set by the empty-value path on unbind', ()=> { + var $rootNode = $(''); + const topics = [{ name: 'somearray' }]; + const $el = $rootNode.find('li:first'); + + // A first render establishes the template snapshot... + repeatHandler.handle([1, 2], 'repeat', $el, topics); + // ...then an empty value hides the element. + repeatHandler.handle([], 'repeat', $el, topics); + $el.is('[hidden]').should.equal(true); + + repeatHandler.unbind('repeat', $el); + $el.is('[hidden]').should.equal(false); + }); }); }); From cc026d2898c6fdb02ce150a93eb8a92c12aa82c5 Mon Sep 17 00:00:00 2001 From: Kai Date: Wed, 22 Jul 2026 15:01:24 -0700 Subject: [PATCH 2/2] TEMPLATE-570 bugfix --- .../attributes/loop-attrs/repeat-attr/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js b/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js index 4d40554e..92c9ac45 100644 --- a/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js +++ b/src/dom/attribute-manager/attributes/loop-attrs/repeat-attr/index.js @@ -25,7 +25,6 @@ const loopAttrHandler = { var id = $el.data(templateIdAttr); if (id) { $el.nextUntil(':not([data-' + id + '])').remove(); - // $el.removeAttr('data-' + templateIdAttr); //FIXME: Something about calling rebind multiple times in IB makes this happen without the removal } const el = $el.get(0); @@ -43,6 +42,11 @@ const loopAttrHandler = { const templateInnerHTML = $(originalHTML).html(); $el.html(templateInnerHTML); $el.removeAttr('hidden'); + // Drop the render-time bookkeeping id so the element is restored to its pristine + // template form. The old replaceWith(originalHTML) removed it implicitly; since we now + // keep the live node we must strip it explicitly. Removed here (after the sibling + // cleanup above, which still needs the id) rather than in the `if (id)` block. + $el.removeAttr('data-' + templateIdAttr); } clearOriginalContents($el); removeKnownData($el);