From 9f9c95b0b3b6ac651df4f78721b9daf34c489a7a Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 13 Dec 2022 13:42:07 +0530 Subject: [PATCH] fix: Incorrect removal of existing elements --- docs/config/setup/modules/mermaidAPI.md | 19 +++--- packages/mermaid/src/mermaidAPI.spec.ts | 85 +++++++++++-------------- packages/mermaid/src/mermaidAPI.ts | 23 +++---- 3 files changed, 53 insertions(+), 74 deletions(-) diff --git a/docs/config/setup/modules/mermaidAPI.md b/docs/config/setup/modules/mermaidAPI.md index 1b840dcd3c..40e4b567f3 100644 --- a/docs/config/setup/modules/mermaidAPI.md +++ b/docs/config/setup/modules/mermaidAPI.md @@ -90,7 +90,7 @@ mermaid.initialize(config); #### Defined in -[mermaidAPI.ts:968](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/mermaidAPI.ts#L968) +[mermaidAPI.ts:961](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/mermaidAPI.ts#L961) ## Functions @@ -295,19 +295,18 @@ Put the svgCode into an iFrame. Return the iFrame code ### removeExistingElements -▸ **removeExistingElements**(`doc`, `isSandboxed`, `id`, `divSelector`, `iFrameSelector`): `void` +▸ **removeExistingElements**(`doc`, `id`, `divId`, `iFrameId`): `void` Remove any existing elements from the given document #### Parameters -| Name | Type | Description | -| :--------------- | :--------- | :---------------------------------------------- | -| `doc` | `Document` | the document to removed elements from | -| `isSandboxed` | `boolean` | whether or not we are in sandboxed mode | -| `id` | `string` | id for any existing SVG element | -| `divSelector` | `string` | selector for any existing enclosing div element | -| `iFrameSelector` | `string` | selector for any existing iFrame element | +| Name | Type | Description | +| :--------- | :--------- | :------------------------------------ | +| `doc` | `Document` | the document to removed elements from | +| `id` | `string` | id for any existing SVG element | +| `divId` | `string` | - | +| `iFrameId` | `string` | - | #### Returns @@ -315,4 +314,4 @@ Remove any existing elements from the given document #### Defined in -[mermaidAPI.ts:336](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/mermaidAPI.ts#L336) +[mermaidAPI.ts:335](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/mermaidAPI.ts#L335) diff --git a/packages/mermaid/src/mermaidAPI.spec.ts b/packages/mermaid/src/mermaidAPI.spec.ts index f9bad66d71..d2fd49f34b 100644 --- a/packages/mermaid/src/mermaidAPI.spec.ts +++ b/packages/mermaid/src/mermaidAPI.spec.ts @@ -470,61 +470,48 @@ describe('mermaidAPI', function () { svgElement.id = svgId; const tempDivElement = givenDocument.createElement('div'); // doesn't matter what the tag is in the test tempDivElement.id = tempDivId; - const tempiFrameElement = givenDocument.createElement('div'); // doesn't matter what the tag is in the test + const tempiFrameElement = givenDocument.createElement('iframe'); // doesn't matter what the tag is in the test tempiFrameElement.id = tempIframeId; it('removes an existing element with given id', () => { rootHtml.appendChild(svgElement); + rootHtml.append(tempDivElement); + rootHtml.append(tempiFrameElement); + expect(givenDocument.getElementById(svgElement.id)).toEqual(svgElement); - removeExistingElements(givenDocument, false, svgId, tempDivId, tempIframeId); + expect(givenDocument.getElementById(tempDivElement.id)).toEqual(tempDivElement); + expect(givenDocument.getElementById(tempiFrameElement.id)).toEqual(tempiFrameElement); + removeExistingElements(givenDocument, svgId, tempDivId, tempIframeId); expect(givenDocument.getElementById(svgElement.id)).toBeNull(); - }); - - describe('is in sandboxed mode', () => { - const inSandboxedMode = true; - - it('removes an existing element with the given iFrame selector', () => { - tempiFrameElement.append(svgElement); - rootHtml.append(tempiFrameElement); - rootHtml.append(tempDivElement); - - expect(givenDocument.getElementById(tempIframeId)).toEqual(tempiFrameElement); - expect(givenDocument.getElementById(tempDivId)).toEqual(tempDivElement); - expect(givenDocument.getElementById(svgId)).toEqual(svgElement); - removeExistingElements( - givenDocument, - inSandboxedMode, - svgId, - '#' + tempDivId, - '#' + tempIframeId - ); - expect(givenDocument.getElementById(tempDivId)).toEqual(tempDivElement); - expect(givenDocument.getElementById(tempIframeId)).toBeNull(); - expect(givenDocument.getElementById(svgId)).toBeNull(); - }); - }); - describe('not in sandboxed mode', () => { - const inSandboxedMode = false; - - it('removes an existing element with the given enclosing div selector', () => { - tempDivElement.append(svgElement); - rootHtml.append(tempDivElement); - rootHtml.append(tempiFrameElement); - - expect(givenDocument.getElementById(tempIframeId)).toEqual(tempiFrameElement); - expect(givenDocument.getElementById(tempDivId)).toEqual(tempDivElement); - expect(givenDocument.getElementById(svgId)).toEqual(svgElement); - removeExistingElements( - givenDocument, - inSandboxedMode, - svgId, - '#' + tempDivId, - '#' + tempIframeId - ); - expect(givenDocument.getElementById(tempIframeId)).toEqual(tempiFrameElement); - expect(givenDocument.getElementById(tempDivId)).toBeNull(); - expect(givenDocument.getElementById(svgId)).toBeNull(); - }); + expect(givenDocument.getElementById(tempDivElement.id)).toBeNull(); + expect(givenDocument.getElementById(tempiFrameElement.id)).toBeNull(); + }); + + it('removes an existing iframe element even if div element is absent', () => { + tempiFrameElement.append(svgElement); + rootHtml.append(tempiFrameElement); + + expect(givenDocument.getElementById(tempIframeId)).toEqual(tempiFrameElement); + expect(givenDocument.getElementById(tempDivId)).toBeNull(); + expect(givenDocument.getElementById(svgId)).toEqual(svgElement); + removeExistingElements(givenDocument, svgId, tempDivId, tempIframeId); + expect(givenDocument.getElementById(tempDivId)).toBeNull(); + expect(givenDocument.getElementById(tempIframeId)).toBeNull(); + expect(givenDocument.getElementById(svgId)).toBeNull(); + }); + + it('removes both existing div and iframe elements when both are present', () => { + tempDivElement.append(svgElement); + rootHtml.append(tempDivElement); + rootHtml.append(tempiFrameElement); + + expect(givenDocument.getElementById(tempIframeId)).toEqual(tempiFrameElement); + expect(givenDocument.getElementById(tempDivId)).toEqual(tempDivElement); + expect(givenDocument.getElementById(svgId)).toEqual(svgElement); + removeExistingElements(givenDocument, svgId, tempDivId, tempIframeId); + expect(givenDocument.getElementById(tempIframeId)).toBeNull(); + expect(givenDocument.getElementById(tempDivId)).toBeNull(); + expect(givenDocument.getElementById(svgId)).toBeNull(); }); }); diff --git a/packages/mermaid/src/mermaidAPI.ts b/packages/mermaid/src/mermaidAPI.ts index a63071364d..5bf11fad18 100644 --- a/packages/mermaid/src/mermaidAPI.ts +++ b/packages/mermaid/src/mermaidAPI.ts @@ -328,29 +328,22 @@ function sandboxedIframe(parentNode: D3Element, iFrameId: string): D3Element { * Remove any existing elements from the given document * * @param doc - the document to removed elements from - * @param isSandboxed - whether or not we are in sandboxed mode * @param id - id for any existing SVG element * @param divSelector - selector for any existing enclosing div element * @param iFrameSelector - selector for any existing iFrame element */ export const removeExistingElements = ( doc: Document, - isSandboxed: boolean, id: string, - divSelector: string, - iFrameSelector: string + divId: string, + iFrameId: string ) => { // Remove existing SVG element if it exists - const existingSvg = doc.getElementById(id); - if (existingSvg) { - existingSvg.remove(); - } - + doc.getElementById(id)?.remove(); // Remove previous temporary element if it exists - const element = isSandboxed ? doc.querySelector(iFrameSelector) : doc.querySelector(divSelector); - if (element) { - element.remove(); - } + // Both div and iframe needs to be cleared in case there is a config change happening between renders. + doc.getElementById(divId)?.remove(); + doc.getElementById(iFrameId)?.remove(); }; /** @@ -443,7 +436,7 @@ const render = function ( // No svgContainingElement was provided // If there is an existing element with the id, we remove it. This likely a previously rendered diagram - removeExistingElements(document, isSandboxed, id, iFrameID_selector, enclosingDivID_selector); + removeExistingElements(document, id, enclosingDivID, iFrameID); // Add the temporary div used for rendering with the enclosingDivID. // This temporary div will contain a svg with the id == id @@ -650,7 +643,7 @@ const renderAsync = async function ( // No svgContainingElement was provided // If there is an existing element with the id, we remove it. This likely a previously rendered diagram - removeExistingElements(document, isSandboxed, id, iFrameID_selector, enclosingDivID_selector); + removeExistingElements(document, id, enclosingDivID, iFrameID); // Add the temporary div used for rendering with the enclosingDivID. // This temporary div will contain a svg with the id == id