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() - 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 }