From 7707d417e50566cf42470860f3f536d765e18ecd Mon Sep 17 00:00:00 2001
From: bjorn <bjorn@swis.nl>
Date: Wed, 29 Jul 2026 21:24:35 +0200
Subject: [PATCH] fix: don't crash when the menu is gone by the time the layer
 click arrives

With `useModal: false` the menu is dismissed by a document-level mousedown
listener rather than by the transparent layer. That listener outlives the
menu it was registered for, so the menu can already be gone when the next
click arrives: a `build` menu empties its own options object once it has
finished hiding, and a `hide` handler calling contextMenu('destroy') tears
it down outright.

handle.layerClick() guarded that case with

    root.$menu === null || typeof root.$menu === 'undefined' || ...

and then called root.$menu.trigger('contextmenu:hide') inside the block it
opened for it, so the very case the guard detected was the one that threw
"Cannot read properties of undefined (reading 'trigger')". The throw also
happened before `onhide` could run, so the dismiss listener was never
unregistered and one more piled up for every menu that had been opened.

Skip straight to the cleanup when there is nothing left to hide, covering
both an absent `$menu` and an empty jQuery object. Everything else on the
path is untouched: the same events fire in the same order, `onhide` keeps
its timing, and the `isNearRecentSelectChange()` grace period still takes
precedence exactly as before.

Fixes #805
---
 CHANGELOG.md                                  |   1 +
 src/jquery.contextMenu.js                     |  27 ++-
 test/specs/issue-805-no-modal-destroy.js      | 109 +++++++++
 ...issue-805-layerclick-after-destroy.test.js | 221 ++++++++++++++++++
 4 files changed, 357 insertions(+), 1 deletion(-)
 create mode 100644 test/specs/issue-805-no-modal-destroy.js
 create mode 100644 test/unit/issue-805-layerclick-after-destroy.test.js

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 918b6628..511c2669 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,7 @@
 * A `select`'s phantom mousedown is no longer treated as an outside click (fixes #744)
 * A re-dispatched layer click now targets the element actually clicked (fixes #771)
 * Guard against undefined `e.data` in the contextmenu handler (fixes #777)
+* Clicking on after the menu was destroyed no longer throws with `useModal: false` (fixes #805)
 
 #### Documentation
 
diff --git a/src/jquery.contextMenu.js b/src/jquery.contextMenu.js
index 5f77c0d9..da8c4636 100644
--- a/src/jquery.contextMenu.js
+++ b/src/jquery.contextMenu.js
@@ -739,8 +739,20 @@
 
                 // if the click closing is done through windwow event listener rather than a transparent layer
                 if (!root.$layer) {
+                    // There may be no menu left to hide at all: this listener
+                    // outlives the menu it was registered for, so the menu can
+                    // already be gone by the time the next click arrives - a
+                    // `build` menu empties its own options object once it has
+                    // finished hiding (see op.hide()), and a `hide` event
+                    // handler calling $(selector).contextMenu('destroy') tears
+                    // it down outright. `$menu` is left either dropped
+                    // altogether or as an empty jQuery object, and the latter
+                    // used to throw on the $menu[0] dereference below.
+                    // See https://github.com/swisnl/jQuery-contextMenu/issues/805
+                    var menuIsGone = !root.$menu || !root.$menu.length;
+
                     target = document.elementFromPoint(x - $win.scrollLeft(), y - $win.scrollTop());
-                    if (root.$menu === null || typeof root.$menu === 'undefined' || (!root.$menu[0].contains(target) && !isWithinDetachedSubmenus(root, target))) {
+                    if (menuIsGone || (!root.$menu[0].contains(target) && !isWithinDetachedSubmenus(root, target))) {
                         // Choosing an option from a native <select> item's dropdown can
                         // make Firefox fire a spurious click/mousedown shortly
                         // afterwards, at coordinates that don't necessarily land within
@@ -756,6 +768,19 @@
                             return;
                         }
 
+                        // Nothing left to hide, so skip straight to the cleanup:
+                        // this used to call root.$menu.trigger() regardless and
+                        // throw "Cannot read properties of undefined (reading
+                        // 'trigger')", which also kept `onhide` from running, so
+                        // op.layer()'s dismiss listener was never unregistered
+                        // and one more of them piled up per menu opened.
+                        // See https://github.com/swisnl/jQuery-contextMenu/issues/805
+                        if (menuIsGone) {
+                            if (typeof onhide !== 'undefined')
+                                onhide();
+                            return;
+                        }
+
                         root.$menu.trigger('contextmenu:hide');
                         if (typeof onhide !== 'undefined')
                             onhide();
diff --git a/test/specs/issue-805-no-modal-destroy.js b/test/specs/issue-805-no-modal-destroy.js
new file mode 100644
index 00000000..2e68d91e
--- /dev/null
+++ b/test/specs/issue-805-no-modal-destroy.js
@@ -0,0 +1,109 @@
+const { test, expect } = require('@playwright/test');
+const { fixture } = require('../support/helpers');
+
+// Regression test for https://github.com/swisnl/jQuery-contextMenu/issues/805
+//
+// With `useModal: false` the menu is dismissed by a document-level mousedown
+// listener instead of by the transparent layer. That listener outlives the menu
+// it belongs to, so the next click has to cope with the menu already being
+// gone - here because a `hide` handler destroys it. handle.layerClick() used to
+// dereference the missing menu, which threw and, since the throw happened
+// before the listener could unregister itself, left one more listener behind
+// for every menu that had been opened.
+
+// Count the dismiss listeners the plugin registers, before any page script runs.
+async function countDismissListeners(page) {
+  await page.addInitScript(() => {
+    window.__dismissListeners = 0;
+    const add = document.addEventListener;
+    const remove = document.removeEventListener;
+    document.addEventListener = function (type, fn, capture) {
+      if (type === 'mousedown' && capture === true) {
+        window.__dismissListeners++;
+      }
+      return add.apply(document, arguments);
+    };
+    document.removeEventListener = function (type, fn, capture) {
+      if (type === 'mousedown' && capture === true) {
+        window.__dismissListeners--;
+      }
+      return remove.apply(document, arguments);
+    };
+  });
+}
+
+// Replace the demo's own menu with one that is dismissed without the modal
+// layer and that destroys itself from its `hide` handler.
+async function setUpMenu(page) {
+  await page.evaluate(() => {
+    const $ = window.jQuery;
+    $.contextMenu('destroy');
+    $.contextMenu({
+      selector: '.context-menu-one',
+      useModal: false,
+      build: function () {
+        return {
+          items: {
+            edit: {name: 'Edit', callback: function () {}},
+            copy: {name: 'Copy', callback: function () {}}
+          }
+        };
+      },
+      events: {
+        hide: function () {
+          $('.context-menu-one').contextMenu('destroy');
+        }
+      }
+    });
+  });
+}
+
+test.describe('Issue 805: clicking on after the menu destroyed itself', () => {
+  test('does not throw and does not leak the dismiss listener', async ({ page }) => {
+    const pageErrors = [];
+    page.on('pageerror', (error) => pageErrors.push(error.message));
+
+    await countDismissListeners(page);
+    await page.goto(fixture('callback.html'));
+    await setUpMenu(page);
+
+    await page.click('.context-menu-one', { button: 'right' });
+    await expect(page.locator('.context-menu-root')).toBeVisible();
+    expect(await page.evaluate(() => window.__dismissListeners)).toBe(1);
+
+    // pick an option, which hides the menu and lets the hide handler destroy it
+    await page.locator('.context-menu-root li').first().click();
+    await expect(page.locator('.context-menu-root')).toHaveCount(0);
+
+    // now click somewhere else entirely, with either button
+    await page.mouse.click(5, 5);
+    await page.mouse.click(5, 5, { button: 'right' });
+
+    expect(pageErrors).toEqual([]);
+    expect(await page.evaluate(() => window.__dismissListeners)).toBe(0);
+  });
+
+  test('leaves the dismiss listeners bounded over repeated open/destroy cycles', async ({ page }) => {
+    const pageErrors = [];
+    page.on('pageerror', (error) => pageErrors.push(error.message));
+
+    await countDismissListeners(page);
+    await page.goto(fixture('callback.html'));
+
+    for (let i = 0; i < 3; i++) {
+      await setUpMenu(page);
+
+      await page.click('.context-menu-one', { button: 'right' });
+      await expect(page.locator('.context-menu-root')).toBeVisible();
+
+      await page.locator('.context-menu-root li').first().click();
+      await expect(page.locator('.context-menu-root')).toHaveCount(0);
+
+      await page.mouse.click(5, 5);
+
+      expect(await page.evaluate(() => window.__dismissListeners)).toBe(0);
+    }
+
+    expect(pageErrors).toEqual([]);
+  });
+});
diff --git a/test/unit/issue-805-layerclick-after-destroy.test.js b/test/unit/issue-805-layerclick-after-destroy.test.js
new file mode 100644
index 00000000..e730b2a9
--- /dev/null
+++ b/test/unit/issue-805-layerclick-after-destroy.test.js
@@ -0,0 +1,221 @@
+// Regression test for https://github.com/swisnl/jQuery-contextMenu/issues/805
+//
+// With `useModal: false` the menu isn't dismissed by a transparent layer but
+// by a document-level mousedown listener (see op.layer()) that calls
+// handle.layerClick() and unregisters itself again through the `onhide`
+// callback it hands in.
+//
+// That listener outlives the menu it was registered for, so by the time the
+// next click arrives there may be no menu left at all: a `build` menu empties
+// its own options object once it has finished hiding, and a `hide` handler
+// calling $(selector).contextMenu('destroy') tears the menu down outright.
+// handle.layerClick() guarded that with
+//
+//     if (root.$menu === null || typeof root.$menu === 'undefined' || !root.$menu[0].contains(target))
+//
+// and then called root.$menu.trigger('contextmenu:hide') inside the block, so
+// the very case the first two clauses detect was the one that threw
+// "Cannot read properties of undefined (reading 'trigger')". Because the throw
+// happened before `onhide` ran, the listener was never removed either and one
+// more accumulated for every menu that had been opened.
+
+(function () {
+  var origAddEventListener = document.addEventListener;
+  var origRemoveEventListener = document.removeEventListener;
+  var liveListeners = [];
+
+  function trackDismissListeners() {
+    liveListeners = [];
+    document.addEventListener = function (type, fn, capture) {
+      if (type === 'mousedown' && capture === true) {
+        liveListeners.push(fn);
+      }
+      return origAddEventListener.apply(document, arguments);
+    };
+    document.removeEventListener = function (type, fn, capture) {
+      if (type === 'mousedown' && capture === true) {
+        var i = liveListeners.indexOf(fn);
+        if (i > -1) {
+          liveListeners.splice(i, 1);
+        }
+      }
+      return origRemoveEventListener.apply(document, arguments);
+    };
+  }
+
+  function stopTrackingDismissListeners() {
+    document.addEventListener = origAddEventListener;
+    document.removeEventListener = origRemoveEventListener;
+    // don't leave the plugin's listeners behind for the next test
+    while (liveListeners.length) {
+      origRemoveEventListener.call(document, 'mousedown', liveListeners.pop(), true);
+    }
+  }
+
+  // Invoke the registered listeners the way the browser would for a click
+  // somewhere outside the menu. Calling them directly (rather than dispatching
+  // the event) keeps a throw inside handle.layerClick() reportable here: a
+  // listener that throws during a real dispatch only reaches window.onerror.
+  function clickOutside(x, y) {
+    var ev = new MouseEvent('mousedown', {
+      bubbles: true,
+      cancelable: true,
+      clientX: x,
+      clientY: y,
+      button: 0
+    });
+    var error = null;
+
+    $.each(liveListeners.slice(), function (i, listener) {
+      try {
+        listener.call(document, ev);
+      } catch (e) {
+        error = e;
+      }
+    });
+
+    return error;
+  }
+
+  function menuOptions(extra) {
+    return $.extend({
+      selector: '#issue805-trigger',
+      useModal: false
+    }, extra);
+  }
+
+  function openAndPickAnItem() {
+    $('#issue805-trigger').contextMenu({x: 210, y: 210});
+    $('.context-menu-list').filter(':visible').find('li').first().trigger('mouseup');
+  }
+
+  QUnit.module('issue 805: outside click after the menu is gone', {
+    beforeEach: function () {
+      trackDismissListeners();
+      // #qunit-fixture is rendered off-screen, where document.elementFromPoint()
+      // would never find anything, so put the trigger on the body instead.
+      $('<div id="issue805-trigger" style="position:fixed;top:200px;left:200px;width:80px;height:40px;"></div>')
+        .appendTo(document.body);
+    },
+    afterEach: function () {
+      stopTrackingDismissListeners();
+      $.contextMenu('destroy');
+      $('#issue805-trigger').remove();
+      $('#context-menu-layer').remove();
+      $('.context-menu-list').remove();
+    }
+  });
+
+  QUnit.test('clicking outside after a build menu tore itself down does not throw', function (assert) {
+    var done = assert.async();
+
+    $.contextMenu(menuOptions({
+      build: function () {
+        return {items: {copy: {name: 'Copy', callback: function () {}}}};
+      }
+    }));
+
+    openAndPickAnItem();
+    assert.equal(liveListeners.length, 1, 'the dismiss listener was registered');
+
+    // wait for the hide animation to complete: that is when a built menu
+    // empties its options object, taking $menu with it
+    setTimeout(function () {
+      var error = clickOutside(10, 10);
+
+      assert.equal(error, null, 'clicking outside did not throw' + (error ? ' (' + error.message + ')' : ''));
+      assert.equal(liveListeners.length, 0, 'the dismiss listener unregistered itself');
+      done();
+    }, 200);
+  });
+
+  QUnit.test('clicking outside after a hide handler destroyed the menu does not throw', function (assert) {
+    var done = assert.async();
+
+    $.contextMenu(menuOptions({
+      build: function () {
+        return {items: {copy: {name: 'Copy', callback: function () {}}}};
+      },
+      events: {
+        hide: function () {
+          $('#issue805-trigger').contextMenu('destroy');
+        }
+      }
+    }));
+
+    openAndPickAnItem();
+
+    setTimeout(function () {
+      var error = clickOutside(10, 10);
+
+      assert.equal(error, null, 'clicking outside did not throw' + (error ? ' (' + error.message + ')' : ''));
+      assert.equal(liveListeners.length, 0, 'the dismiss listener unregistered itself');
+      done();
+    }, 200);
+  });
+
+  QUnit.test('clicking outside with an emptied $menu does not throw either', function (assert) {
+    var done = assert.async();
+    var root = null;
+
+    $.contextMenu(menuOptions({
+      items: {copy: {name: 'Copy', callback: function () {}}},
+      events: {
+        hide: function (opt) {
+          root = opt;
+        }
+      }
+    }));
+
+    openAndPickAnItem();
+
+    setTimeout(function () {
+      assert.ok(root, 'the hide handler ran');
+      // $menu can be left as an empty jQuery object rather than dropped
+      // altogether, which the old guard did not cover at all
+      root.$menu = $();
+
+      var error = clickOutside(10, 10);
+
+      assert.equal(error, null, 'clicking outside did not throw' + (error ? ' (' + error.message + ')' : ''));
+      assert.equal(liveListeners.length, 0, 'the dismiss listener unregistered itself');
+      done();
+    }, 200);
+  });
+
+  QUnit.test('the dismiss listeners do not accumulate over repeated open/hide cycles', function (assert) {
+    var done = assert.async();
+    var cycles = 3;
+
+    function cycle(remaining) {
+      if (remaining === 0) {
+        assert.equal(liveListeners.length, 0, 'no dismiss listeners are left behind');
+        return done();
+      }
+
+      // the hide handler destroys the registration, so set it up every round
+      $.contextMenu(menuOptions({
+        build: function () {
+          return {items: {copy: {name: 'Copy', callback: function () {}}}};
+        },
+        events: {
+          hide: function () {
+            $('#issue805-trigger').contextMenu('destroy');
+          }
+        }
+      }));
+
+      openAndPickAnItem();
+
+      setTimeout(function () {
+        var error = clickOutside(10, 10);
+
+        assert.equal(error, null, 'cycle ' + (cycles - remaining + 1) + ' did not throw');
+        assert.equal(liveListeners.length, 0, 'cycle ' + (cycles - remaining + 1) + ' left no dismiss listener behind');
+        cycle(remaining - 1);
+      }, 200);
+    }
+
+    cycle(cycles);
+  });
+})();