fix: Removing attribute when mouseover node causes error TG-440

tolgee · Dec 7, 2021 · dde937f · dde937f
1 parent 624d36c
commit dde937f
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 27 deletions.
diff --git a/packages/core/src/helpers/NodeHelper.ts b/packages/core/src/helpers/NodeHelper.ts
@@ -2,22 +2,6 @@ import { ArgumentTypes } from './commonTypes';
 import { TOLGEE_TARGET_ATTRIBUTE } from '../Constants/Global';
 
 export class NodeHelper {
-  private static *evaluateGenerator<T extends Node>(
-    expression: string,
-    targetNode: Node
-  ): Generator<T> {
-    let node: Node;
-    const evaluated = document.evaluate(
-      expression,
-      targetNode,
-      undefined,
-      XPathResult.ANY_TYPE
-    );
-    while ((node = evaluated.iterateNext()) !== null) {
-      yield node as T;
-    }
-  }
-
   static evaluate<T extends Node>(
     ...args: ArgumentTypes<typeof NodeHelper.evaluateGenerator>
   ): T[] {
@@ -71,11 +55,28 @@ export class NodeHelper {
     }
     if (node instanceof Attr) {
       const ownerContainsAttr =
+        node.ownerElement &&
         Object.values(node.ownerElement.attributes).indexOf(node) > -1;
       if (descendant.contains(node.ownerElement) && ownerContainsAttr) {
         return true;
       }
     }
     return false;
   }
+
+  private static *evaluateGenerator<T extends Node>(
+    expression: string,
+    targetNode: Node
+  ): Generator<T> {
+    let node: Node;
+    const evaluated = document.evaluate(
+      expression,
+      targetNode,
+      undefined,
+      XPathResult.ANY_TYPE
+    );
+    while ((node = evaluated.iterateNext()) !== null) {
+      yield node as T;
+    }
+  }
 }
diff --git a/packages/core/src/highlighter/MouseEventHandler.test.ts b/packages/core/src/highlighter/MouseEventHandler.test.ts
@@ -64,6 +64,22 @@ describe('MouseEventHandler', () => {
       expect(mockedHighlight).toBeCalledTimes(0);
     });
 
+    test('Will prevent clean on mouseover', async () => {
+      mockedElement.dispatchEvent(mockedMouseOut);
+      window.dispatchEvent(mockedKeyup);
+      mockedHighlight = jest.fn();
+      mockedElement.dispatchEvent(mockedMouseOver);
+      expect(mockedElement._tolgee.preventClean).toBeTruthy();
+    });
+
+    test('Will remove prevent clean on mouseout', async () => {
+      mockedElement.dispatchEvent(mockedMouseOut);
+      window.dispatchEvent(mockedKeyup);
+      mockedElement._tolgee.preventClean = true;
+      mockedElement.dispatchEvent(mockedMouseOut);
+      expect(mockedElement._tolgee.preventClean).toBeFalsy();
+    });
+
     test('Will not highlight just on keydown', async () => {
       window.dispatchEvent(mockedKeyup);
       mockedElement.dispatchEvent(mockedMouseOut);

diff --git a/packages/core/src/highlighter/MouseEventHandler.ts b/packages/core/src/highlighter/MouseEventHandler.ts
@@ -101,12 +101,14 @@ export class MouseEventHandler {
   }
 
   private onMouseOut(element) {
+    element._tolgee.preventClean = false;
     this.mouseOn.delete(element);
     this.mouseOnChanged.emit(this.getMouseOn());
   }
 
   private onMouseOver(element: ElementWithMeta & ElementCSSInlineStyle) {
     this.filterMouseOn();
+    element._tolgee.preventClean = true;
     this.mouseOn.delete(element); //to get in to last place
     this.mouseOn.add(element);
     this.mouseOnChanged.emit(this.getMouseOn());

diff --git a/packages/core/src/services/ElementRegistrar.test.ts b/packages/core/src/services/ElementRegistrar.test.ts
@@ -104,6 +104,23 @@ describe('ElementRegistrar', () => {
         expect(mockedElement._tolgee).not.toBeDefined();
       }
     });
+
+    test("clean all doesn't clean elements with preventClean", () => {
+      mockedElements[1]._tolgee.preventClean = true;
+      const node = mockedElements[1]._tolgee.nodes.values().next().value;
+      node.parentElement.removeChild(node);
+      elementRegistrar.refreshAll();
+      expect(mockedElements[1]._tolgee).toBeDefined();
+      expect(mockedElements[1]).toHaveAttribute(TOLGEE_ATTRIBUTE_NAME);
+    });
+
+    test("refresh all doesn't delete nodes on elements wih preventClean", () => {
+      mockedElements[1]._tolgee.preventClean = true;
+      const node = mockedElements[1]._tolgee.nodes.values().next().value;
+      node.parentElement.removeChild(node);
+      elementRegistrar.refreshAll();
+      expect(mockedElements[1]._tolgee).toBeDefined();
+    });
   });
 
   test('will register attribute node', () => {

diff --git a/packages/core/src/services/ElementRegistrar.ts b/packages/core/src/services/ElementRegistrar.ts
@@ -37,12 +37,14 @@ export class ElementRegistrar {
 
   refreshAll() {
     for (const element of this.registeredElements) {
-      this.cleanElementInactiveNodes(element);
-      if (
-        element._tolgee.nodes.size === 0 &&
-        !element._tolgee.wrappedWithElementOnlyKey
-      ) {
-        this.cleanElement(element);
+      if (!element._tolgee.preventClean) {
+        this.cleanElementInactiveNodes(element);
+        if (
+          element._tolgee.nodes.size === 0 &&
+          !element._tolgee.wrappedWithElementOnlyKey
+        ) {
+          this.cleanElement(element);
+        }
       }
     }
   }
@@ -82,12 +84,14 @@ export class ElementRegistrar {
   }
 
   private cleanElement(element: ElementWithMeta) {
-    if (typeof element._tolgee.removeAllEventListeners === 'function') {
-      element._tolgee.removeAllEventListeners();
+    if (!element._tolgee.preventClean) {
+      if (typeof element._tolgee.removeAllEventListeners === 'function') {
+        element._tolgee.removeAllEventListeners();
+      }
+      element.removeAttribute(TOLGEE_ATTRIBUTE_NAME);
+      delete element._tolgee;
+      this.registeredElements.delete(element);
     }
-    element.removeAttribute(TOLGEE_ATTRIBUTE_NAME);
-    delete element._tolgee;
-    this.registeredElements.delete(element);
   }
 
   private *getActiveNodes(element: ElementWithMeta) {

diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
@@ -63,6 +63,17 @@ export type ElementMeta = {
   highlight?: () => void;
   unhighlight?: () => void;
   initialBackgroundColor?: string;
+  /**
+   * Stops removing of element's inactive nodes and
+   * unregistering from ElementRegistrar.
+   *
+   * It's used when user has mouse on the element, so there is
+   * potential, that element highlight will be triggered.
+   *
+   * Triggering highlight needs the metadata stored on element, so
+   * we need the ability to prevent clean.
+   */
+  preventClean?: boolean;
 };
 
 export type NodeMeta = {