I have this bug with 7.x-1.x, but it exists in 8.x-1.x and 8.x-2.x. I think it is customary to first fix the bug in the most recent branch.

modal.js contains the following code:

    $(document).bind('keydown', modalEventEscapeCloseHandler);
...
    function close() {
...
      $('body').unbind('keypress', modalEventEscapeCloseHandler);

The bug here is that the event is bound to document.keydown, but unbound from body.keypress. This will leave the event handler after the modal is closed and create multiple event handlers when multiple modals are opened during a pageview.

The bug was introduced together with the feature in commit 0ec61b4f340981bb365b975661c99993676e9add and changed in commit 93cd187ccee28ade17da308d4012c28e724fc535.

I'll attach a patch shortly.

CommentFileSizeAuthor
#1 ctools-modal-keypress-event-2444347-1.patch892 bytesJorrit

Comments

Jorrit’s picture

Jorrit commented
Status: Active » Needs review
StatusFileSize
new ctools-modal-keypress-event-2444347-1.patch892 bytes

The patch.

Status: Needs review » Needs work

The last submitted patch, 1: ctools-modal-keypress-event-2444347-1.patch, failed testing.

Jorrit’s picture

Jorrit commented
Version: 8.x-2.x-dev » 8.x-1.x-dev
Status: Needs work » Needs review

Jorrit queued 1: ctools-modal-keypress-event-2444347-1.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: ctools-modal-keypress-event-2444347-1.patch, failed testing.

Jorrit’s picture

Jorrit commented
Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Needs work » Needs review

Jorrit queued 1: ctools-modal-keypress-event-2444347-1.patch for re-testing.

Jorrit’s picture

Jorrit commented

It seems the branches 8.x-1.x and 8.x-2.x are not testable right now. The patch applies to all branches, so no backporting is needed.

Jorrit’s picture

Jorrit commented
Assigned: Jorrit » Unassigned
chris matthews’s picture

chris matthews commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

The 4 year old patch to modal.js does not apply to the latest ctools 7.x-1.x-dev and if still applicable needs to be re-rolled.

Checking patch js/modal.js...
error: while searching for:
      $('body').unbind( 'focus', modalEventHandler);
      $('body').unbind( 'keypress', modalEventHandler );
      $('.close').unbind('click', modalContentClose);
      $('body').unbind('keypress', modalEventEscapeCloseHandler);
      $(document).trigger('CToolsDetachBehaviors', $('#modalContent'));

      // Set our animation parameters and use them

error: patch failed: js/modal.js:459
error: js/modal.js: patch does not apply

  • joelpittet committed ebfc944 on 7.x-1.x authored by Jorrit 
    Issue #2444347 by Jorrit: modal.js unbinds wrong keydown event
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Fixed
Issue tags: -Needs reroll

There is no javascript files in the 8.x branch but thanks I've committed this to the 7.x branch.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Parent issue: » #2984057: Plan for CTools 7.x-1.15 release

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.