From 3fad3b550782bb21016321a907f538d6bd298105 Mon Sep 17 00:00:00 2001 From: Ori Riner Date: Thu, 5 Oct 2017 13:36:27 +0300 Subject: [PATCH 1/2] clear previous children when SVG node doesn't have innerHTML --- src/renderers/dom/shared/setInnerHTML.js | 3 ++ .../utils/__tests__/setInnerHTML-test.js | 47 +++++++++++++++---- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/renderers/dom/shared/setInnerHTML.js b/src/renderers/dom/shared/setInnerHTML.js index fd320769d2251..4a749ab3be441 100644 --- a/src/renderers/dom/shared/setInnerHTML.js +++ b/src/renderers/dom/shared/setInnerHTML.js @@ -31,6 +31,9 @@ var setInnerHTML = createMicrosoftUnsafeLocalFunction(function(node, html) { reusableSVGContainer || document.createElement('div'); reusableSVGContainer.innerHTML = '' + html + ''; var svgNode = reusableSVGContainer.firstChild; + while (node.firstChild) { + node.removeChild(node.firstChild); + } while (svgNode.firstChild) { node.appendChild(svgNode.firstChild); } diff --git a/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js b/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js index fe625a6f0b374..deb0c1e5b4e4a 100644 --- a/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js +++ b/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js @@ -24,17 +24,35 @@ describe('setInnerHTML', () => { }); describe('when the node does not have an innerHTML property', () => { - // Disabled. JSDOM doesn't seem to remove nodes when using appendChild to - // move existing nodes. - xit('sets innerHTML on it', () => { + var node; + var nodeProxy; + beforeEach(() => { // Create a mock node that looks like an SVG in IE (without innerHTML) - var node = { - namespaceURI: Namespaces.svg, - appendChild: jasmine.createSpy(), - }; + node = document.createElementNS(Namespaces.svg, 'svg'); + nodeProxy = new Proxy(node, { + has: (target, prop) => { + return prop === 'innerHTML' ? false : prop in target; + }, + get: (target, prop) => { + if (prop === 'appendChild') { + return function(child) { + child.parentElement.removeChild(child); + return target.appendChild(child); + }; + } else { + return target[prop]; + } + }, + }); + + spyOn(node, 'appendChild').and.callThrough(); + spyOn(node, 'removeChild').and.callThrough(); + }); + + it('sets innerHTML on it', () => { var html = ''; - setInnerHTML(node, html); + setInnerHTML(nodeProxy, html); expect(node.appendChild.calls.argsFor(0)[0].outerHTML).toBe( '', @@ -43,5 +61,18 @@ describe('setInnerHTML', () => { '', ); }); + + it('clears previous children', () => { + var firstHtml = ''; + var secondHtml = ''; + setInnerHTML(nodeProxy, firstHtml); + + setInnerHTML(nodeProxy, secondHtml); + + expect(node.removeChild.calls.argsFor(0)[0].outerHTML).toBe( + '', + ); + expect(node.innerHTML).toBe(''); + }); }); }); From 9786d9ae021f7967d6102e4ed92f5f49c32af5fa Mon Sep 17 00:00:00 2001 From: Ori Riner Date: Tue, 10 Oct 2017 09:25:23 +0300 Subject: [PATCH 2/2] remove 'get' handler for appendChild in test node proxy --- .../dom/shared/utils/__tests__/setInnerHTML-test.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js b/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js index deb0c1e5b4e4a..6b2cd5b6ac97c 100644 --- a/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js +++ b/src/renderers/dom/shared/utils/__tests__/setInnerHTML-test.js @@ -34,16 +34,6 @@ describe('setInnerHTML', () => { has: (target, prop) => { return prop === 'innerHTML' ? false : prop in target; }, - get: (target, prop) => { - if (prop === 'appendChild') { - return function(child) { - child.parentElement.removeChild(child); - return target.appendChild(child); - }; - } else { - return target[prop]; - } - }, }); spyOn(node, 'appendChild').and.callThrough();