fix: left-click trigger leaked a synthetic contextmenu event to unrelated ancestor listeners (#793)

* fix: stop left-click/hover/touchstart triggers from leaking a synthetic contextmenu event

When trigger is 'left' (or 'hover'/'touchstart'), the plugin simulated the
native right-click by doing $(this).trigger($.Event('contextmenu', ...)).
jQuery's trigger() bubbles that synthetic event through every ancestor's
jQuery-bound 'contextmenu' handlers, not just the plugin's own delegated
listener. Any unrelated code on the page listening for 'contextmenu' (e.g.
a grid widget's own right-click handler) would fire on a plain left-click,
indistinguishable from a real right-click.

Call the internal dispatcher (handle.contextmenu) directly instead of
triggering a bubbling event, so the synthetic event never reaches handlers
outside the plugin.

Fixes #754

* fix: guard hover-trigger dispatch against a trigger removed during its delay

The hover trigger's contextmenu dispatch is delayed by e.data.delay
(default 200ms). Calling the dispatcher directly instead of triggering
a bubbling event (see the previous commit) means the delayed callback
no longer no-ops if the trigger element got removed from the document
in the meantime - it would now run against a detached node.

Bail out of the timeout callback if the trigger is no longer attached
to the document.

Author: bbrala

src/jquery.contextMenu.js

@@ -416,7 +416,19 @@
             click: function (e) {
                 e.preventDefault();
                 e.stopImmediatePropagation();
-                $(this).trigger($.Event('contextmenu', {data: e.data, pageX: e.pageX, pageY: e.pageY}));
+                // Invoke the dispatcher directly instead of triggering a real,
+                // bubbling 'contextmenu' event: a bubbling synthetic event is
+                // indistinguishable from a genuine right-click to any other
+                // 'contextmenu' listener bound on an ancestor element, causing
+                // unrelated handlers elsewhere on the page to fire on a plain
+                // left-click. See https://github.com/swisnl/jQuery-contextMenu/issues/754
+                handle.contextmenu.call(this, $.Event('contextmenu', {
+                    data: e.data,
+                    pageX: e.pageX,
+                    pageY: e.pageY,
+                    target: this,
+                    currentTarget: this
+                }));
             },
             // contextMenu right-click trigger
             mousedown: function (e) {
@@ -441,7 +453,15 @@
                     e.preventDefault();
                     e.stopImmediatePropagation();
                     $currentTrigger = $this;
-                    $this.trigger($.Event('contextmenu', {data: e.data, pageX: e.pageX, pageY: e.pageY}));
+                    // See handle.click for why we call the dispatcher directly
+                    // instead of triggering a bubbling 'contextmenu' event.
+                    handle.contextmenu.call(this, $.Event('contextmenu', {
+                        data: e.data,
+                        pageX: e.pageX,
+                        pageY: e.pageY,
+                        target: this,
+                        currentTarget: this
+                    }));
                 }
 
                 $this.removeData('contextMenuActive');
@@ -469,11 +489,25 @@
                 hoveract.timer = setTimeout(function () {
                     hoveract.timer = null;
                     $document.off('mousemove.contextMenuShow');
+
+                    // The trigger may have been removed from the document while
+                    // this delay was pending. Triggering a bubbling event used to
+                    // make that a silent no-op (a detached node can't bubble to
+                    // the delegated listener); calling the dispatcher directly
+                    // doesn't have that safety net, so check explicitly.
+                    if (!$.contains(document.documentElement, $this[0])) {
+                        return;
+                    }
+
                     $currentTrigger = $this;
-                    $this.trigger($.Event('contextmenu', {
+                    // See handle.click for why we call the dispatcher directly
+                    // instead of triggering a bubbling 'contextmenu' event.
+                    handle.contextmenu.call($this[0], $.Event('contextmenu', {
                         data: hoveract.data,
                         pageX: hoveract.pageX,
-                        pageY: hoveract.pageY
+                        pageY: hoveract.pageY,
+                        target: $this[0],
+                        currentTarget: $this[0]
                     }));
                 }, e.data.delay);
             },

test/unit/issue-754-repro.test.js

@@ -0,0 +1,75 @@
+QUnit.module('issue 754 repro', {
+  afterEach: function() {
+    $.contextMenu('destroy');
+    var $fixture = $('#qunit-fixture');
+    if ($fixture.length) {
+      $fixture.html('');
+    }
+  }
+});
+
+QUnit.test('left-click trigger should not bubble a synthetic contextmenu event to unrelated ancestor handlers', function(assert) {
+  var $fixture = $('#qunit-fixture');
+  if ($fixture.length === 0) {
+    $('<div id="qunit-fixture">').appendTo('body');
+    $fixture = $('#qunit-fixture');
+  }
+
+  $fixture.append('<div class="outer-grid"><span class="left-trigger">left click me</span></div>');
+
+  var outerContextMenuCount = 0;
+  $('.outer-grid').on('contextmenu', function() {
+    outerContextMenuCount++;
+  });
+
+  var menuOpenCount = 0;
+  $.contextMenu({
+    selector: '.left-trigger',
+    trigger: 'left',
+    events: {
+      show: function() { menuOpenCount++; }
+    },
+    items: {
+      copy: {name: 'Copy'}
+    }
+  });
+
+  $('.left-trigger').trigger($.Event('click', {which: 1, button: 0}));
+
+  assert.equal(menuOpenCount, 1, 'sanity check: our own contextMenu opened once');
+  assert.equal(outerContextMenuCount, 0, 'unrelated ancestor contextmenu handler should NOT fire on left-click trigger');
+});
+
+QUnit.test('hover trigger should not dispatch against a trigger removed from the document during its delay', function(assert) {
+  var done = assert.async();
+  var $fixture = $('#qunit-fixture');
+  if ($fixture.length === 0) {
+    $('<div id="qunit-fixture">').appendTo('body');
+    $fixture = $('#qunit-fixture');
+  }
+
+  $fixture.append('<span class="hover-trigger">hover me</span>');
+  var $trigger = $('.hover-trigger');
+
+  var menuOpenCount = 0;
+  $.contextMenu({
+    selector: '.hover-trigger',
+    trigger: 'hover',
+    delay: 10,
+    events: {
+      show: function() { menuOpenCount++; }
+    },
+    items: {
+      copy: {name: 'Copy'}
+    }
+  });
+
+  $trigger.trigger($.Event('mouseenter', {pageX: 0, pageY: 0}));
+  // remove the trigger from the document before the hover delay elapses
+  $trigger.remove();
+
+  setTimeout(function() {
+    assert.equal(menuOpenCount, 0, 'menu should not open for a trigger that was removed before the delay elapsed');
+    done();
+  }, 30);
+});