When a new OffCanvas is opened, it executes Drupal.offCanvas.afterCreate to register few events and it only unregisters these event when you clicked the CLOSE button which fires 

Drupal.offCanvas.beforeClose

However, in Setting Tray's Editing mode, it allows clicking thought different triggers, highlighted blocks, to edit different target element. Another new OffCanvas opens a new dialog via Ajax. It executes `openDialog` command and it replaces the current dialog contents without calling `Drupal.offCanvas.afterCreate`. It left zombie event handlers. It grows super fast (2^n + 1)!

Take handleDialogResize as an example, it called:

click: times
1 click: 1
2 click: 4
3 click: 9
4 click: 16

To test click on different blocks to open the off-canvas dialog but don't close the dialog before clicking the next block.

CommentFileSizeAuthor
#7 2902429-7.patch3.66 KBGrandmaGlassesRopeMan
#7 interdiff-2902429-4-7.txt1.28 KBGrandmaGlassesRopeMan
#4 2902429-4.patch4.4 KBGrandmaGlassesRopeMan
offCanvasEvents.patch4.42 KBdroplet

Comments

droplet created an issue. See original summary.

droplet’s picture

droplet commented
Issue summary: View changes
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Title: Remove previous event handlers » Remove previous event handlers when opening new off-canvas block edit form in the off-canvas dialog
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

@droplet thanks for finding this!

I manually tested this by adding a JS breakpoint in handleDialogResize(). I saw the behavior the @droplet was referring to and confirmed that the patch fixes the problem.

GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
commented
StatusFileSize
new 2902429-4.patch4.4 KB

- rerolled due to #2902304: Remove `scroll.off-canvas` event

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Great catch!

star-szr’s picture

star-szr
he/him
Primary language English
commented 
+++ b/core/modules/outside_in/js/off-canvas.es6.js
@@ -90,7 +105,6 @@
       const eventData = { settings, $element, offCanvasDialog: this };
-
       $element

Minor but seems like this blank line should be kept.

GrandmaGlassesRopeMan’s picture

GrandmaGlassesRopeMan
he/him
Primary language English
Location 🌎
commented
StatusFileSize
new interdiff-2902429-4-7.txt1.28 KB
new 2902429-7.patch3.66 KB

#6 Yep, 👍🍕

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Ok, since it looks like committer feedback was addressed, and this has remained RTBC for a few days after...

Committed and pushed to 8.5.x, cherry-picked to 8.4.x. Thanks!

  • webchick committed fcf01cc on 8.5.x 
    Issue #2902429 by drpal, droplet, tedbow: Remove previous event handlers...

  • webchick committed 8f750f7 on 8.4.x 
    Issue #2902429 by drpal, droplet, tedbow: Remove previous event handlers...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Component: outside_in.module » settings_tray.module

Changing to new settings_tray.module component. @drpal thanks for script help! :)

Status: Fixed » Closed (fixed)

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