fix: Incorrect removal of existing elements

mermaid-js · Dec 13, 2022 · 9f9c95b · 9f9c95b
1 parent 3f0b13a
commit 9f9c95b
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 74 deletions.
diff --git 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,24 +295,23 @@ 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
 
 `void`
 
 #### 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
@@ -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
@@ -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