Dxclick: nodes-disposing

packages/devextreme/js/__internal/events/__tests__/click.test.ts

@@ -0,0 +1,69 @@
+import {
+  afterEach, describe, expect, it, jest,
+} from '@jest/globals';
+import { removeEvent } from '@js/common/core/events/remove';
+import eventsEngine from '@ts/events/core/m_events_engine';
+
+import { name as clickEventName } from '../click';
+
+type ElementEventData = Record<string, { handleObjects: unknown[] } | undefined>;
+
+const noop = (): void => {};
+
+const getRemoveHandlersCount = (element: Element): number => {
+  const elementData = eventsEngine.elementDataMap.get(element) as ElementEventData | undefined;
+
+  return elementData?.[removeEvent]?.handleObjects.length ?? 0;
+};
+
+describe('dxclick nodes disposing (5025)', () => {
+  const clickableNodes: HTMLElement[] = [];
+
+  const createClickableNode = (): HTMLElement => {
+    const node = document.createElement('div');
+
+    document.body.appendChild(node);
+    eventsEngine.on(node, clickEventName, noop);
+    clickableNodes.push(node);
+
+    return node;
+  };
+
+  afterEach(() => {
+    clickableNodes.forEach((node) => {
+      eventsEngine.off(node);
+      node.remove();
+    });
+    clickableNodes.length = 0;
+  });
+
+  it('keeps a foreign dxremove handler on the previously clicked node', () => {
+    const clicked = createClickableNode();
+    const other = createClickableNode();
+
+    clicked.click();
+
+    const foreignHandler = jest.fn();
+    eventsEngine.on(clicked, removeEvent, foreignHandler);
+
+    other.click();
+    eventsEngine.triggerHandler(clicked, { type: removeEvent });
+
+    expect(foreignHandler).toHaveBeenCalledTimes(1);
+  });
+
+  it('removes only its own dxremove handler from the previously clicked node', () => {
+    const clicked = createClickableNode();
+    const other = createClickableNode();
+
+    clicked.click();
+    expect(getRemoveHandlersCount(clicked)).toBe(1);
+
+    eventsEngine.on(clicked, removeEvent, noop);
+    expect(getRemoveHandlersCount(clicked)).toBe(2);
+
+    other.click();
+
+    expect(getRemoveHandlersCount(clicked)).toBe(1);
+  });
+});

packages/devextreme/js/__internal/events/click.ts

@@ -28,10 +28,11 @@ interface NodesDisposingSubscription {
 
 let prevented: boolean | null = null;
 let lastFiredEvent: NativeClickEvent | null = null;
-const subscriptions = new Map<NativeClickEvent, NodesDisposingSubscription>();
+let lastSubscription: NodesDisposingSubscription | null = null;
 
 const onNodeRemove = (): void => {
   lastFiredEvent = null;
+  lastSubscription = null;
 };
 
 const clickHandler = function (e: EmitterEvent & { originalEvent: NativeClickEvent }): void {
@@ -45,25 +46,19 @@ const clickHandler = function (e: EmitterEvent & { originalEvent: NativeClickEve
       originalEvent.DXCLICK_FIRED = true;
     }
 
-    if (lastFiredEvent && subscriptions.has(lastFiredEvent)) {
-      // @ts-expect-error the subscription stores onceCallback, not callback, so this
-      // destructured callback is always undefined and off() drops every dxremove
-      // handler from the nodes
-      const { nodes, callback } = subscriptions.get(lastFiredEvent) as NodesDisposingSubscription;
+    if (lastFiredEvent && lastSubscription) {
+      const { nodes, onceCallback } = lastSubscription;
 
-      unsubscribeNodesDisposing(lastFiredEvent, callback, nodes);
+      unsubscribeNodesDisposing(lastFiredEvent, onceCallback, nodes);
 
-      subscriptions.delete(lastFiredEvent);
+      lastSubscription = null;
     }
 
     lastFiredEvent = originalEvent;
 
-    const subscriptionData: NodesDisposingSubscription = subscribeNodesDisposing(
-      lastFiredEvent,
-      onNodeRemove,
-    );
-
-    subscriptions.set(lastFiredEvent, subscriptionData);
+    if (originalEvent) {
+      lastSubscription = subscribeNodesDisposing(originalEvent, onNodeRemove);
+    }
 
     fireEvent({
       type: CLICK_EVENT_NAME,

packages/devextreme/testing/tests/DevExpress.ui.events/click.tests.js

@@ -1,6 +1,7 @@
 import $ from 'jquery';
 import { noop } from 'core/utils/common';
 import clickEvent from 'common/core/events/click';
+import { removeEvent } from 'common/core/events/remove';
 import domUtils from '__internal/core/utils/m_dom';
 import support from '__internal/core/utils/m_support';
 import devices from '__internal/core/m_devices';
@@ -425,3 +426,21 @@ QUnit.test('dxclick should not be fired twice when \'click\' is triggered from i
     pointer.start().down().up();
     $(document).off('dxclick', $.noop);
 });
+
+QUnit.test('foreign dxremove handler on the previously clicked node should survive a dxclick on another node (5025)', function(assert) {
+    const $clicked = $('#first').on('dxclick', noop);
+    const $other = $('#second').on('dxclick', noop);
+    let foreignHandlerCallCount = 0;
+
+    nativePointerMock($clicked).start().click();
+
+    $clicked.on(removeEvent, function() {
+        foreignHandlerCallCount++;
+    });
+
+    nativePointerMock($other).start().click();
+
+    $clicked.triggerHandler({ type: removeEvent });
+
+    assert.equal(foreignHandlerCallCount, 1, `foreign ${removeEvent} handler is still subscribed`);
+});

packages/devextreme/testing/tests/DevExpress.ui.events/events.utils.nodesDisposing.tests.js

@@ -31,7 +31,7 @@ QUnit.test('should clean elementDataMap when using subscribeNodesDisposing and u
         ? afterSubscribeElementData[removeEvent].handleObjects.length
         : 0;
 
-    unsubscribeNodesDisposing(clickEvent, subscriptionData.callback, subscriptionData.nodes);
+    unsubscribeNodesDisposing(clickEvent, subscriptionData.onceCallback, subscriptionData.nodes);
 
     const finalElementData = eventsEngine.elementDataMap.get(document);
     const afterUnsubscribeHandleObjectsCount = finalElementData && finalElementData[removeEvent]