[stable5.7] fix(drag-and-drop): track all droppable-mailbox directive instances independently

src/directives/drag-and-drop/droppable-mailbox/droppable-mailbox.js

@@ -6,11 +6,20 @@ import logger from '../../../logger.js'
 import dragEventBus from '../util/dragEventBus.js'
 
 export class DroppableMailbox {
-	constructor(el, componentInstance, options) {
+	constructor(el, options) {
 		this.el = el
 		this.options = options
 		this.mainStore = options.mainStore
-		this.registerListeners.bind(this)(el)
+
+		// Store bound references so removeListeners can use the same
+		// function references that were passed to addEventListener/on
+		this._onDragStart = this.onDragStart.bind(this)
+		this._onDragEnd = this.onDragEnd.bind(this)
+		this._onDragOver = this.onDragOver.bind(this)
+		this._onDragLeave = this.onDragLeave.bind(this)
+		this._onDrop = this.onDrop.bind(this)
+
+		this.registerListeners(el)
 		this.setInitialAttributes()
 	}
 
@@ -19,30 +28,25 @@ export class DroppableMailbox {
 		this.setStatus('enabled')
 	}
 
-	update(el, instance) {
-		this.setInitialAttributes()
-		this.options = instance.options
-	}
-
 	registerListeners(el) {
-		dragEventBus.on('drag-start', this.onDragStart.bind(this))
-		dragEventBus.on('drag-end', this.onDragEnd.bind(this))
+		dragEventBus.on('drag-start', this._onDragStart)
+		dragEventBus.on('drag-end', this._onDragEnd)
 
 		// event listeners need to be attached to the first child element
 		// (a button or an anchor tag) instead of the root el, because there
 		// can be sub-mailboxes within the root element of the directive
-		el.firstChild.addEventListener('dragover', this.onDragOver.bind(this))
-		el.firstChild.addEventListener('dragleave', this.onDragLeave.bind(this))
-		el.firstChild.addEventListener('drop', this.onDrop.bind(this))
+		el.firstChild.addEventListener('dragover', this._onDragOver)
+		el.firstChild.addEventListener('dragleave', this._onDragLeave)
+		el.firstChild.addEventListener('drop', this._onDrop)
 	}
 
 	removeListeners(el) {
-		dragEventBus.off('drag-start', this.onDragStart)
-		dragEventBus.off('drag-end', this.onDragEnd)
+		dragEventBus.off('drag-start', this._onDragStart)
+		dragEventBus.off('drag-end', this._onDragEnd)
 
-		el.firstChild.removeEventListener('dragover', this.onDragOver)
-		el.firstChild.removeEventListener('dragleave', this.onDragLeave)
-		el.firstChild.removeEventListener('drop', this.onDrop)
+		el.firstChild.removeEventListener('dragover', this._onDragOver)
+		el.firstChild.removeEventListener('dragleave', this._onDragLeave)
+		el.firstChild.removeEventListener('drop', this._onDrop)
 	}
 
 	setStatus(status) {

src/directives/drag-and-drop/droppable-mailbox/index.js

@@ -4,16 +4,37 @@
  */
 import { DroppableMailbox } from './droppable-mailbox.js'
 
-let instance
+const instances = new WeakMap()
 
-export const DroppableMailboxDirective = {
-	bind(el, binding, vnode) {
-		instance = new DroppableMailbox(el, vnode.context, binding.value)
-	},
-	componentUpdated(el, binding) {
+function onBind(el, binding) {
+	const instance = new DroppableMailbox(el, binding.value)
+	instances.set(el, instance)
+}
+
+function onUpdate(el, binding) {
+	const instance = instances.get(el)
+	if (instance) {
 		instance.options = binding.value
-		instance.update(el, instance)
-	},
+	}
+}
+
+function onUnbind(el) {
+	const instance = instances.get(el)
+	if (instance) {
+		instance.removeListeners(el)
+	}
+	instances.delete(el)
+}
+
+export const DroppableMailboxDirective = {
+	// Vue 2
+	bind: onBind,
+	componentUpdated: onUpdate,
+	unbind: onUnbind,
+	// Vue 3
+	mounted: onBind,
+	updated: onUpdate,
+	unmounted: onUnbind,
 }
 
 export default DroppableMailbox

src/tests/unit/directives/droppable-mailbox.spec.js

@@ -0,0 +1,168 @@
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { DroppableMailboxDirective } from '../../../directives/drag-and-drop/droppable-mailbox/index.js'
+import dragEventBus from '../../../directives/drag-and-drop/util/dragEventBus.js'
+
+/**
+ * Creates a mock DOM element with a firstChild for the directive.
+ *
+ * @param {string} id - Identifier for the element
+ * @return {object} Mock element
+ */
+function createMockEl(id) {
+	const firstChild = {
+		addEventListener: vi.fn(),
+		removeEventListener: vi.fn(),
+	}
+	return { id, firstChild, setAttribute: vi.fn() }
+}
+
+/**
+ * Creates a mock binding value for the directive.
+ *
+ * @param {object} overrides - Override default options
+ * @return {object} Mock binding
+ */
+function createBinding(overrides = {}) {
+	return {
+		value: {
+			mainStore: {},
+			mailboxId: 1,
+			accountId: 1,
+			isValidDropTarget: true,
+			...overrides,
+		},
+	}
+}
+
+describe('DroppableMailboxDirective', () => {
+	// Use the Vue 2 hooks since the project is on Vue 2.7
+	const { bind, componentUpdated, unbind } = DroppableMailboxDirective
+
+	let boundEls = []
+
+	afterEach(() => {
+		boundEls.forEach((el) => unbind(el))
+		boundEls = []
+	})
+
+	it('tracks multiple instances independently', () => {
+		const el1 = createMockEl('el1')
+		const el2 = createMockEl('el2')
+		const el3 = createMockEl('el3')
+
+		bind(el1, createBinding({ mailboxId: 1 }))
+		bind(el2, createBinding({ mailboxId: 2 }))
+		bind(el3, createBinding({ mailboxId: 3 }))
+		boundEls.push(el1, el2, el3)
+
+		// All elements should have the expected event listeners registered
+		for (const el of [el1, el2, el3]) {
+			expect(el.firstChild.addEventListener).toHaveBeenCalledWith(
+				'dragover',
+				expect.any(Function),
+			)
+			expect(el.firstChild.addEventListener).toHaveBeenCalledWith(
+				'dragleave',
+				expect.any(Function),
+			)
+			expect(el.firstChild.addEventListener).toHaveBeenCalledWith(
+				'drop',
+				expect.any(Function),
+			)
+		}
+	})
+
+	it('updates the correct instance on componentUpdated', () => {
+		const el1 = createMockEl('el1')
+		const el2 = createMockEl('el2')
+
+		bind(el1, createBinding({ mailboxId: 1, isValidDropTarget: true }))
+		bind(el2, createBinding({ mailboxId: 2, isValidDropTarget: true }))
+		boundEls.push(el1, el2)
+
+		// Update el1 to disable drop target
+		componentUpdated(
+			el1,
+			createBinding({ mailboxId: 1, isValidDropTarget: false }),
+		)
+
+		// Clear setAttribute calls from bind so we only see drag-start effects
+		el1.setAttribute.mockClear()
+		el2.setAttribute.mockClear()
+
+		// Trigger drag-start on all instances via the event bus
+		dragEventBus.emit('drag-start', { accountId: 1, mailboxId: 99 })
+
+		// el1 should be disabled because its isValidDropTarget was set to false
+		expect(el1.setAttribute).toHaveBeenCalledWith(
+			'droppable-mailbox',
+			'disabled',
+		)
+
+		// el2 should NOT be disabled because its options were not changed
+		expect(el2.setAttribute).not.toHaveBeenCalledWith(
+			'droppable-mailbox',
+			'disabled',
+		)
+	})
+
+	it('removes listeners and instance on unbind', () => {
+		const el1 = createMockEl('el1')
+		const el2 = createMockEl('el2')
+
+		bind(el1, createBinding({ mailboxId: 1 }))
+		bind(el2, createBinding({ mailboxId: 2 }))
+		boundEls.push(el2)
+
+		unbind(el1)
+
+		// el1 should have had removeEventListener called for each event
+		expect(el1.firstChild.removeEventListener).toHaveBeenCalledWith(
+			'dragover',
+			expect.any(Function),
+		)
+		expect(el1.firstChild.removeEventListener).toHaveBeenCalledWith(
+			'dragleave',
+			expect.any(Function),
+		)
+		expect(el1.firstChild.removeEventListener).toHaveBeenCalledWith(
+			'drop',
+			expect.any(Function),
+		)
+
+		// el2 should NOT have had removeEventListener called
+		expect(el2.firstChild.removeEventListener).not.toHaveBeenCalled()
+	})
+
+	it('removes the same listener references that were added', () => {
+		const el = createMockEl('el1')
+
+		bind(el, createBinding({ mailboxId: 1 }))
+		unbind(el)
+
+		// Each function passed to removeEventListener should be the same
+		// reference that was passed to addEventListener. If .bind() is called
+		// twice instead of storing the reference, these will be different
+		// functions and the listener will leak.
+		const added = el.firstChild.addEventListener.mock.calls
+		const removed = el.firstChild.removeEventListener.mock.calls
+
+		for (let i = 0; i < 3; i++) {
+			const [addEvent, addFn] = added[i]
+			const [removeEvent, removeFn] = removed[i]
+			expect(removeEvent).toBe(addEvent)
+			expect(removeFn).toBe(addFn)
+		}
+	})
+
+	it('handles unbind of non-existent element gracefully', () => {
+		const el = createMockEl('unknown')
+
+		// Should not throw
+		expect(() => unbind(el)).not.toThrow()
+	})
+})