From 38b2455c77deb092bfb07b1ddf899defb104f85b Mon Sep 17 00:00:00 2001
From: skjnldsv <skjnldsv@protonmail.com>
Date: Tue, 29 Sep 2026 21:49:44 +0200
Subject: [PATCH] fix(handlers): take the same handler registered twice quietly

Every app bundles its own copy of the package, so the server's copy and
an app's both register the default handlers, and each duplicate logged
a warning. A repeat registration with the same id and tagname is now
skipped at debug level, keeping the first. Another handler claiming a
taken id still warns.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
---
 README.md                         |  3 +++
 __tests__/fileActions.spec.ts     |  3 ++-
 __tests__/registerHandler.spec.ts | 42 +++++++++++++++++++++++++++++++
 lib/handlers.ts                   | 13 ++++++++--
 4 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 __tests__/registerHandler.spec.ts

diff --git a/README.md b/README.md
index d6a4a19..92c2555 100644
--- a/README.md
+++ b/README.md
@@ -175,6 +175,9 @@ Gotchas:
   it is not namespaced for you. A collision does not throw: the second
   registration is silently dropped with a console warning, so pick something
   specific to your app (`myapp-image`, not `image`).
+- Registering the same handler again, with the same `id` and `tagname`, is
+  quietly ignored. That is what happens when several copies of the package
+  on a page register the defaults, so there is nothing to guard against.
 - Registering after the viewer has already read the handler list is not an
   error either — the handler just never appears in the "Open with …" menu.
   See [step 3](#3-load-your-registration-before-the-viewer) below for why
diff --git a/__tests__/fileActions.spec.ts b/__tests__/fileActions.spec.ts
index 5712998..5840c40 100644
--- a/__tests__/fileActions.spec.ts
+++ b/__tests__/fileActions.spec.ts
@@ -129,7 +129,8 @@ describe('registerHandler registry', () => {
 		const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {})
 
 		registerHandler(makeHandler({ id: 'dup' }))
-		registerHandler(makeHandler({ id: 'dup' }))
+		// Another handler, not the same one registered twice
+		registerHandler(makeHandler({ id: 'dup', tagname: 'other-app-dup' }))
 
 		expect(warn).toHaveBeenCalledTimes(1)
 		expect(warn).toHaveBeenCalledWith(expect.stringContaining('dup'))
diff --git a/__tests__/registerHandler.spec.ts b/__tests__/registerHandler.spec.ts
new file mode 100644
index 0000000..0848840
--- /dev/null
+++ b/__tests__/registerHandler.spec.ts
@@ -0,0 +1,42 @@
+/*!
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { registerHandler } from '../lib/handlers.ts'
+import { scope } from '../lib/scope.ts'
+import { logger } from '../lib/services/logger.ts'
+import { makeHandler } from './factories.ts'
+
+describe('registering a handler whose id is taken', () => {
+	afterEach(() => {
+		vi.restoreAllMocks()
+	})
+
+	it('quietly keeps the first when it is the same handler again', () => {
+		// Several copies of the package on a page each register the
+		// defaults: that is one handler, not a collision
+		const warn = vi.spyOn(logger, 'warn')
+		const first = makeHandler({ id: 'repeat', tagname: 'oca-viewer-repeat' })
+		registerHandler(first)
+		registerHandler(makeHandler({ id: 'repeat', tagname: 'oca-viewer-repeat' }))
+
+		expect(scope.handlers!.get('repeat')).toBe(first)
+		expect(warn).not.toHaveBeenCalled()
+	})
+
+	it('lets a second copy of the package register the defaults without a word', async () => {
+		// The server registers them on every page, and an app bundling its
+		// own copy for older servers asks again
+		const { registerDefaultHandlers } = await import('../lib/defaults.ts')
+		registerDefaultHandlers()
+		vi.resetModules()
+		const second = await import('../lib/defaults.ts')
+		const { logger: secondLogger } = await import('../lib/services/logger.ts')
+		const warn = vi.spyOn(secondLogger, 'warn')
+
+		second.registerDefaultHandlers()
+
+		expect(warn).not.toHaveBeenCalled()
+	})
+})
diff --git a/lib/handlers.ts b/lib/handlers.ts
index 899fddc..a43bd40 100644
--- a/lib/handlers.ts
+++ b/lib/handlers.ts
@@ -211,8 +211,17 @@ export function registerHandler(handler: IHandler): void {
 	validateHandler(handler)
 
 	scope.handlers ??= new Map<string, IHandler>()
-	if (scope.handlers.has(handler.id)) {
-		logger.warn(`Handler with id ${handler.id} is already registered.`)
+	const registered = scope.handlers.get(handler.id)
+	if (registered !== undefined) {
+		// Every app bundles its own copy of the package, so the same handler
+		// can be registered more than once: the server's copy and an app's
+		// both register the defaults. The first one stays, as the custom
+		// element its tagname names is the first copy's too.
+		if (registered.tagname === handler.tagname) {
+			logger.debug(`Handler ${handler.id} is already registered, keeping the first registration`)
+		} else {
+			logger.warn(`Handler with id ${handler.id} is already registered for <${registered.tagname}>, ignoring the one for <${handler.tagname}>.`)
+		}
 		return
 	}