From 4800408da8d4b65d26a37a2f41ecd6c27549443b Mon Sep 17 00:00:00 2001 From: jefbinomed Date: Tue, 11 Aug 2026 14:34:06 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20fix(touch-pointer):=20calcul?= =?UTF-8?q?er=20le=20zoom=20dans=20le=20rep=C3=A8re=20du=20frame=20(#41)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le zoom du plugin touch-pointer calculait sa translation à partir de window.innerWidth/innerHeight alors qu'il applique le transform sur #slideViewFrame. En vue presenter, le frame n'occupe que 3/5 de la largeur : mesuré avant correctif, la slide était projetée en x=835..2339 pour une fenêtre de 1280, soit entièrement hors de l'écran. - extraction de la géométrie dans touch-pointer/zoom.ts, testable sans layout (jsdom n'en calcule pas) : t = s · (centre − point), dans le repère du frame ; - plafonnement de la translation à (s − 1) · taille / 2, pour que le frame agrandi recouvre toujours sa boîte — sans quoi un clic en coin décolle un bord et laisse apparaître du vide ; - _toggleZoom reçoit désormais les pourcentages bruts et les convertit contre le rect du frame, plus contre la fenêtre ; - overflow: hidden sur #slideViewSection en filet de sécurité. Tests : 8 cas unitaires sur la fonction pure (dont une propriété de recouvrement balayant toutes les positions de clic), 3 cas sur _toggleZoom, et un e2e golden-path vérifié rouge sans le correctif. --- e2e/golden-path/14-plugins.spec.js | 34 ++++++++ .../web-components/slide-view/slide-view.ts | 8 ++ src/plugins/input/touch-pointer/index.ts | 43 ++++++---- src/plugins/input/touch-pointer/zoom.ts | 53 ++++++++++++ .../plugins/input/touch-pointer/index.spec.ts | 62 +++++++++---- test/plugins/input/touch-pointer/zoom.spec.ts | 86 +++++++++++++++++++ 6 files changed, 252 insertions(+), 34 deletions(-) create mode 100644 src/plugins/input/touch-pointer/zoom.ts create mode 100644 test/plugins/input/touch-pointer/zoom.spec.ts diff --git a/e2e/golden-path/14-plugins.spec.js b/e2e/golden-path/14-plugins.spec.js index 80a47031..70c08bdd 100644 --- a/e2e/golden-path/14-plugins.spec.js +++ b/e2e/golden-path/14-plugins.spec.js @@ -13,4 +13,38 @@ test.describe('Golden-path 7 — Menu plugins', () => { // Le plugin manuel (non autoActivate) touchPointerInput doit être proposé. await expect(page.getByText('touchPointerInput', { exact: false })).toBeVisible(); }); + + // Régression #41 : le zoom du touch pointer calculait sa translation dans le repère + // de la FENÊTRE alors qu'il transforme l'iframe, qui n'occupe que 3/5 de la largeur + // en vue presenter. Résultat mesuré avant correctif : frame projeté en x=835..2339 + // pour une fenêtre de 1280 — slide entièrement hors écran. + test('le zoom du touch pointer garde la slide dans son cadre', async ({ page }) => { + await openView(page, 'presenter.html'); + + await page.getByText('Plugins', { exact: false }).click(); + await page.getByText('touchPointerInput', { exact: false }).click(); + + const mask = page.locator('tc-touch-pointer-mask'); + await expect(mask).toBeVisible(); + + // Clic près du coin haut-gauche : le cas le plus défavorable, celui qui + // projetait la slide le plus loin hors du cadre. + await mask.click({ position: { x: 30, y: 20 } }); + + await expect + .poll(async () => + page.evaluate(() => { + const slide = document.getElementById('currentSlide'); + const root = slide.shadowRoot; + const frame = root.getElementById('slideViewFrame').getBoundingClientRect(); + const section = root.getElementById('slideViewSection').getBoundingClientRect(); + return { + zoomed: root.getElementById('slideViewFrame').style.transform.includes('scale'), + // Le frame agrandi doit recouvrir sa section, sans laisser de vide. + covers: frame.left <= section.left && frame.top <= section.top && frame.right >= section.right && frame.bottom >= section.bottom + }; + }) + ) + .toEqual({ zoomed: true, covers: true }); + }); }); diff --git a/src/client/web-components/slide-view/slide-view.ts b/src/client/web-components/slide-view/slide-view.ts index bf756b82..0b5e4cea 100644 --- a/src/client/web-components/slide-view/slide-view.ts +++ b/src/client/web-components/slide-view/slide-view.ts @@ -20,6 +20,14 @@ export class SlideViewComponent extends TcStyledElement { width: 100%; height: 100%; } + /* Filet de sécurité du zoom (#41) : le plugin touch-pointer applique un + transform scale() sur l'iframe. Sans clipping, une translation erronée + projette la slide par-dessus le reste de la vue presenter — voire hors + de l'écran. La géométrie est corrigée dans touch-pointer/zoom.ts ; ceci + garantit qu'aucune régression future ne puisse repeindre hors du cadre. */ + section { + overflow: hidden; + } section.fullscreen { width: 100vw; height: 100vh; diff --git a/src/plugins/input/touch-pointer/index.ts b/src/plugins/input/touch-pointer/index.ts index a043019a..7a8ab25e 100644 --- a/src/plugins/input/touch-pointer/index.ts +++ b/src/plugins/input/touch-pointer/index.ts @@ -1,5 +1,6 @@ import { Plugin } from '@plugins/plugin.js'; import { logger } from '@services/logger'; +import { ZOOM_SCALE, computeZoomTransform } from './zoom'; interface PointerState { x: string; @@ -155,10 +156,10 @@ class TouchPointerInput extends Plugin { } if (messageData.type === 'pointerClick') { - this._toggleZoom( - this._convertPercentToCoordinates(messageData.payload.x ?? '0', window.innerWidth), - this._convertPercentToCoordinates(messageData.payload.y ?? '0', window.innerHeight) - ); + // Les coordonnées arrivent en POURCENTAGE de la zone de masque : elles ne sont + // converties en pixels qu'à l'intérieur de _toggleZoom, dans le repère du frame + // (cf. zoom.ts). Les convertir ici contre la fenêtre était le bug #41. + this._toggleZoom(messageData.payload.x ?? '0', messageData.payload.y ?? '0'); } } @@ -192,7 +193,7 @@ class TouchPointerInput extends Plugin { } } - _toggleZoom(mouseX: number, mouseY: number): void { + _toggleZoom(xPercent: string, yPercent: string): void { const currentSlide = document.getElementById('currentSlide'); if (!currentSlide || !currentSlide.shadowRoot) { return; @@ -204,23 +205,29 @@ class TouchPointerInput extends Plugin { } if (this.zooming) { - element.style.transform = 'translate3D(0px, 0px, 0px)'; - element.style.cursor = 'zoom-in'; - this.zooming = !this.zooming; + this._resetZoom(element); return; } - const scaleValue = 2; - const windowCenterX = window.innerWidth / 2; - const windowCenterY = window.innerHeight / 2; - const targetX = Math.round((windowCenterX - mouseX) * scaleValue); - const targetY = Math.round((windowCenterY - mouseY) * scaleValue); + // getBoundingClientRect() plutôt que offsetWidth : on ne passe ici qu'en état + // dézoomé, donc le rect vaut la boîte de mise en page, et il tient compte d'un + // éventuel scale porté par un ancêtre. + const box = element.getBoundingClientRect(); + const point = { + x: this._convertPercentToCoordinates(xPercent, box.width), + y: this._convertPercentToCoordinates(yPercent, box.height) + }; + const translation = computeZoomTransform(box, point, ZOOM_SCALE); - if (!this.zooming) { - element.style.transform = `translateX(${targetX}px) translateY(${targetY}px) scale(${scaleValue})`; - element.style.cursor = 'zoom-out'; - this.zooming = !this.zooming; - } + element.style.transform = `translateX(${Math.round(translation.x)}px) translateY(${Math.round(translation.y)}px) scale(${ZOOM_SCALE})`; + element.style.cursor = 'zoom-out'; + this.zooming = true; + } + + _resetZoom(element: HTMLElement): void { + element.style.transform = 'translate3D(0px, 0px, 0px)'; + element.style.cursor = 'zoom-in'; + this.zooming = false; } _convertPercentToCoordinates(percentValue: string, size: number): number { diff --git a/src/plugins/input/touch-pointer/zoom.ts b/src/plugins/input/touch-pointer/zoom.ts new file mode 100644 index 00000000..71a2a498 --- /dev/null +++ b/src/plugins/input/touch-pointer/zoom.ts @@ -0,0 +1,53 @@ +/** + * Géométrie du zoom du plugin touch-pointer, isolée du DOM pour être testable + * sans layout (jsdom n'en calcule pas). + * + * Le zoom applique `translate(t) scale(s)` sur `#slideViewFrame`, dont le + * `transform-origin` est le centre (valeur par défaut). Un point `p` exprimé + * dans le repère LOCAL du frame (origine = coin haut-gauche du frame) se + * retrouve donc en `c + s·(p − c) + t`, avec `c` le centre du frame. + * + * Pour amener le point cliqué au centre, on résout `c = c + s·(p − c) + t`, + * d'où `t = s·(c − p)`. + * + * ⚠️ Le repère est celui du FRAME, pas celui de la fenêtre : c'était la cause + * du bug #41. En vue presenter, le frame n'occupe que 3/5 de la largeur, et + * calculer `t` à partir de `window.innerWidth` projetait la slide entièrement + * hors de l'écran. + */ + +export const ZOOM_SCALE = 2; + +export interface Size { + width: number; + height: number; +} + +export interface Point { + x: number; + y: number; +} + +function clamp(value: number, bound: number): number { + return Math.min(Math.max(value, -bound), bound); +} + +/** + * Translation à appliquer pour centrer `point` dans un frame de taille `box` + * agrandi d'un facteur `scale`. + * + * Le résultat est plafonné pour que le frame agrandi recouvre toujours sa boîte + * d'origine : au-delà de `(scale − 1)·taille / 2`, un bord se décollerait du + * conteneur, laissant apparaître du vide (et débordant de l'autre côté). + * + * @param box - dimensions de mise en page du frame, en pixels + * @param point - point à centrer, dans le repère local du frame, en pixels + * @param scale - facteur d'agrandissement (1 = pas de zoom) + * @returns translation en pixels à appliquer avant le `scale` + */ +export function computeZoomTransform(box: Size, point: Point, scale: number): Point { + return { + x: clamp(scale * (box.width / 2 - point.x), ((scale - 1) * box.width) / 2), + y: clamp(scale * (box.height / 2 - point.y), ((scale - 1) * box.height) / 2) + }; +} diff --git a/test/plugins/input/touch-pointer/index.spec.ts b/test/plugins/input/touch-pointer/index.spec.ts index b25e337b..263913f9 100644 --- a/test/plugins/input/touch-pointer/index.spec.ts +++ b/test/plugins/input/touch-pointer/index.spec.ts @@ -272,18 +272,16 @@ describe('TouchPointerInput', function () { expect(setPointerColorSpy).toHaveBeenCalledWith(''); }); - it('should toggle the zoom with converted coordinates on "pointerClick"', function () { - // Given + it('should forward the raw percentages to the zoom on "pointerClick"', function () { + // Given la conversion en pixels se fait dans _toggleZoom, contre le repère du + // frame et non celui de la fenêtre (#41). const toggleZoomSpy = vi.spyOn(instance, '_toggleZoom'); - vi.stubGlobal('innerWidth', 1000); - vi.stubGlobal('innerHeight', 500); // When instance._onMessageEvent(messageEvent({ type: 'x', data: { type: 'pointerClick', payload: { x: '50%', y: '20%' } } })); // Then - expect(toggleZoomSpy).toHaveBeenCalledWith(500, 100); - vi.unstubAllGlobals(); + expect(toggleZoomSpy).toHaveBeenCalledWith('50%', '20%'); }); it('should default missing coordinates to "0" on "pointerClick"', function () { @@ -294,7 +292,7 @@ describe('TouchPointerInput', function () { instance._onMessageEvent(messageEvent({ type: 'x', data: { type: 'pointerClick', payload: {} } })); // Then - expect(toggleZoomSpy).toHaveBeenCalledWith(0, 0); + expect(toggleZoomSpy).toHaveBeenCalledWith('0', '0'); }); }); @@ -387,8 +385,13 @@ describe('TouchPointerInput', function () { }); describe('_toggleZoom()', function () { + /** jsdom ne calcule aucun layout : il faut simuler la boîte du frame. */ + function giveFrameABox(frame: HTMLElement, width: number, height: number): void { + vi.spyOn(frame, 'getBoundingClientRect').mockReturnValue({ width, height } as DOMRect); + } + it('should do nothing when there is no current slide', function () { - expect(() => instance._toggleZoom(1, 2)).not.toThrow(); + expect(() => instance._toggleZoom('1%', '2%')).not.toThrow(); }); it('should do nothing when slideViewFrame is missing', function () { @@ -399,32 +402,59 @@ describe('TouchPointerInput', function () { host.attachShadow({ mode: 'open' }); // When / Then - expect(() => instance._toggleZoom(1, 2)).not.toThrow(); + expect(() => instance._toggleZoom('1%', '2%')).not.toThrow(); }); it('should zoom in and flip the zooming flag when not zoomed', function () { // Given const { frame } = buildCurrentSlide(); - vi.stubGlobal('innerWidth', 1000); - vi.stubGlobal('innerHeight', 500); + giveFrameABox(frame, 800, 600); - // When - instance._toggleZoom(400, 200); + // When un clic au centre du frame + instance._toggleZoom('50%', '50%'); - // Then + // Then aucune translation nécessaire, seulement l'agrandissement + expect(frame.style.transform).toBe('translateX(0px) translateY(0px) scale(2)'); expect(frame.style.cursor).toBe('zoom-out'); - expect(frame.style.transform).toContain('translateX'); expect(instance.zooming).toBe(true); + }); + + it('should compute the translation in the frame reference frame, not the window one', function () { + // Given une fenêtre bien plus large que le frame : c'est la configuration de + // la vue presenter, où le frame n'occupe que 3/5 de la largeur (#41). + const { frame } = buildCurrentSlide(); + giveFrameABox(frame, 800, 600); + vi.stubGlobal('innerWidth', 4000); + vi.stubGlobal('innerHeight', 3000); + + // When + instance._toggleZoom('25%', '25%'); + + // Then t = 2 * (400 - 200) = 400, plafonné à (2 - 1) * 800 / 2 = 400 + // t = 2 * (300 - 150) = 300, plafonné à (2 - 1) * 600 / 2 = 300 + expect(frame.style.transform).toBe('translateX(400px) translateY(300px) scale(2)'); vi.unstubAllGlobals(); }); + it('should never translate the frame off its own box, even on a corner click', function () { + // Given + const { frame } = buildCurrentSlide(); + giveFrameABox(frame, 800, 600); + + // When un clic dans le coin, qui demanderait 2 * 400 = 800 sans plafond + instance._toggleZoom('0%', '0%'); + + // Then plafonné au recouvrement : la slide reste visible (#41) + expect(frame.style.transform).toBe('translateX(400px) translateY(300px) scale(2)'); + }); + it('should zoom out and flip the zooming flag when already zoomed', function () { // Given const { frame } = buildCurrentSlide(); instance.zooming = true; // When - instance._toggleZoom(0, 0); + instance._toggleZoom('0%', '0%'); // Then expect(frame.style.cursor).toBe('zoom-in'); diff --git a/test/plugins/input/touch-pointer/zoom.spec.ts b/test/plugins/input/touch-pointer/zoom.spec.ts new file mode 100644 index 00000000..f27b9bd6 --- /dev/null +++ b/test/plugins/input/touch-pointer/zoom.spec.ts @@ -0,0 +1,86 @@ +import { ZOOM_SCALE, computeZoomTransform } from '@plugins/input/touch-pointer/zoom'; + +describe('computeZoomTransform()', function () { + describe('centrage du point cliqué', function () { + it('should not translate at all when the click is dead center', function () { + // Given / When + const transform = computeZoomTransform({ width: 800, height: 600 }, { x: 400, y: 300 }, 2); + + // Then + expect(transform).toEqual({ x: 0, y: 0 }); + }); + + it('should translate towards the center, proportionally to the scale', function () { + // Given un clic en deçà du centre, assez proche pour rester sous le plafond + // de recouvrement (400 pour la largeur, 300 pour la hauteur à scale 2). + // When + const transform = computeZoomTransform({ width: 800, height: 600 }, { x: 300, y: 250 }, 2); + + // Then t = s * (centre - point) : 2 * 100 et 2 * 50 + expect(transform).toEqual({ x: 200, y: 100 }); + }); + + it('should translate in the opposite direction for a click past the center', function () { + // Given / When + const transform = computeZoomTransform({ width: 800, height: 600 }, { x: 500, y: 350 }, 2); + + // Then + expect(transform).toEqual({ x: -200, y: -100 }); + }); + }); + + describe('recouvrement du conteneur (clamp)', function () { + // Le frame agrandi doit TOUJOURS couvrir son conteneur : sinon il déborde d'un côté + // (bug #41, où la slide sortait entièrement de l'écran) et découvre du vide de l'autre. + it('should clamp the translation so a corner click keeps the frame covering its box', function () { + // Given un clic dans le coin haut-gauche, qui demanderait t = 2 * 400 = 800 + // When + const transform = computeZoomTransform({ width: 800, height: 600 }, { x: 0, y: 0 }, 2); + + // Then plafonné à (s - 1) * taille / 2 + expect(transform).toEqual({ x: 400, y: 300 }); + }); + + it('should clamp symmetrically for the opposite corner', function () { + // Given / When + const transform = computeZoomTransform({ width: 800, height: 600 }, { x: 800, y: 600 }, 2); + + // Then + expect(transform).toEqual({ x: -400, y: -300 }); + }); + + it('should keep the transformed frame within its own box for any click position', function () { + // Given + const box = { width: 752, height: 668 }; + const scale = ZOOM_SCALE; + + for (let ratio = 0; ratio <= 1; ratio += 0.05) { + // When + const point = { x: box.width * ratio, y: box.height * ratio }; + const { x, y } = computeZoomTransform(box, point, scale); + + // Then les bords du frame transformé encadrent bien [0, taille] + const left = (box.width / 2) * (1 - scale) + x; + const right = (box.width / 2) * (1 + scale) + x; + const top = (box.height / 2) * (1 - scale) + y; + const bottom = (box.height / 2) * (1 + scale) + y; + + expect(left).toBeLessThanOrEqual(0); + expect(right).toBeGreaterThanOrEqual(box.width); + expect(top).toBeLessThanOrEqual(0); + expect(bottom).toBeGreaterThanOrEqual(box.height); + } + }); + }); + + describe('cas dégénérés', function () { + it('should return a neutral transform for a zero-sized box', function () { + // jsdom ne calcule pas de layout : getBoundingClientRect() y renvoie des zéros. + expect(computeZoomTransform({ width: 0, height: 0 }, { x: 0, y: 0 }, 2)).toEqual({ x: 0, y: 0 }); + }); + + it('should return a neutral transform when the scale is 1 (no zoom)', function () { + expect(computeZoomTransform({ width: 800, height: 600 }, { x: 0, y: 0 }, 1)).toEqual({ x: 0, y: 0 }); + }); + }); +}); From d423236510737e9402c72453d6cf0fa7b9a73d51 Mon Sep 17 00:00:00 2001 From: jefbinomed Date: Tue, 11 Aug 2026 14:38:45 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20fix(plugins):=20rendre=20le?= =?UTF-8?q?=20cycle=20init/unload=20sym=C3=A9trique=20(#42)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le pointeur quittait bien le DOM à la désactivation — le symptôme décrit dans l'issue est corrigé depuis le passage au shadow DOM — mais le reste du teardown ne suivait pas. Mesuré sur l'app réelle, trois cycles activer/désactiver faisaient grimper `callbacks` à 1, 2 puis 3 : chaque événement du plugin repartait en autant d'exemplaires. - Plugin.unload() (classe de base) purge les callbacks et repasse `initialized` à false ; les surcharges appellent super.unload() ; - touch-pointer : clearInterval du timer de masquage, restauration des styles posés sur le frame (dimensions, transition, curseur) et remise à zéro du transform — désactiver en état zoomé laissait sinon la slide hors cadre, sans retour possible ; - touch-pointer : le handler `message` était enregistré via une lambda inline, donc impossible à détacher — référence stabilisée ; - keyboard et touch : même traitement. Ils n'étaient pas concernés tant que unload() était un no-op, mais la base remet désormais `initialized` à false : sans détacher, une réactivation doublerait leurs listeners. Tests : contrat de la classe de base, 6 cas de teardown sur touch-pointer, détachement des listeners sur keyboard et touch, et une régression sur pluginService couvrant trois cycles complets. --- src/plugins/input/keyboard/index.ts | 17 +++- src/plugins/input/touch-pointer/index.ts | 39 +++++++- src/plugins/input/touch/index.ts | 15 ++- src/plugins/plugin.ts | 15 ++- test/common/services/plugin.spec.ts | 24 +++++ test/plugins/input/keyboard/index.spec.ts | 32 +++++++ .../plugins/input/touch-pointer/index.spec.ts | 91 +++++++++++++++++++ test/plugins/input/touch/index.spec.ts | 19 ++++ test/plugins/plugin.spec.ts | 47 ++++++++++ 9 files changed, 292 insertions(+), 7 deletions(-) create mode 100644 test/plugins/plugin.spec.ts diff --git a/src/plugins/input/keyboard/index.ts b/src/plugins/input/keyboard/index.ts index 574d46c6..5745724a 100644 --- a/src/plugins/input/keyboard/index.ts +++ b/src/plugins/input/keyboard/index.ts @@ -2,19 +2,32 @@ import { config } from '@services/config'; import { Plugin } from '@plugins/plugin.js'; class KeyboardInput extends Plugin { + // Références stables : sans elles, removeEventListener n'a aucune prise et + // `unload()` laisserait les listeners actifs derrière lui (#42). + private readonly onKeyUp = (event: KeyboardEvent): void => this._captureKeyboardEvent(event, true); + private readonly onKeyDown = (event: KeyboardEvent): void => this._captureKeyboardEvent(event); + constructor() { super(); this.type = 'inputEvent'; } override init(): void { - addEventListener('keyup', e => this._captureKeyboardEvent(e, true), true); + addEventListener('keyup', this.onKeyUp, true); // keydown sans forward : ne sert qu'à stopPropagation sur les touches bloquées, // avant que la présentation embarquée ne les voie. Seul keyup déclenche la navigation. - addEventListener('keydown', this._captureKeyboardEvent.bind(this), true); + addEventListener('keydown', this.onKeyDown, true); this.initialized = true; } + override unload(): void { + // Le 3e argument doit reproduire le flag de capture utilisé à l'ajout, + // sinon le navigateur ne retrouve pas le listener. + removeEventListener('keyup', this.onKeyUp, true); + removeEventListener('keydown', this.onKeyDown, true); + super.unload(); + } + _captureKeyboardEvent(event: KeyboardEvent, forward = false): void { const keys = config.tcComponent.keysBlocked; const activeElementIsInput = document.activeElement?.tagName && /input|textarea/i.test(document.activeElement.tagName); diff --git a/src/plugins/input/touch-pointer/index.ts b/src/plugins/input/touch-pointer/index.ts index 7a8ab25e..a1b5d1b2 100644 --- a/src/plugins/input/touch-pointer/index.ts +++ b/src/plugins/input/touch-pointer/index.ts @@ -26,6 +26,9 @@ class TouchPointerInput extends Plugin { interval: ReturnType | undefined; messageEventRegistered = false; + /** Référence stable, sans laquelle `removeEventListener` serait impossible. */ + private readonly messageHandler = (message: MessageEvent): void => this._onMessageEvent(message); + constructor() { super(); this.type = 'touchPointerEvent'; @@ -37,7 +40,7 @@ class TouchPointerInput extends Plugin { this._addPointer(); if (!this.messageEventRegistered) { - addEventListener('message', message => this._onMessageEvent(message)); + addEventListener('message', this.messageHandler); this.messageEventRegistered = true; } this.initialized = true; @@ -47,7 +50,39 @@ class TouchPointerInput extends Plugin { this._removeSettingsArea(); this._removeMaskArea(); this._removePointer(); - this.initialized = false; + this._restoreFrame(); + + if (this.messageEventRegistered) { + removeEventListener('message', this.messageHandler); + this.messageEventRegistered = false; + } + + clearInterval(this.interval); + this.interval = undefined; + + super.unload(); + } + + /** Shadow root de la slide courante, ou `undefined` hors vue presenter/on-stage. */ + _slideShadowRoot(): ShadowRoot | undefined { + return document.getElementById('currentSlide')?.shadowRoot ?? undefined; + } + + /** + * Rend au frame l'apparence qu'il avait avant `_addPointer` : dimensions, + * transition, curseur de zoom et transform éventuel. Sans cela, désactiver + * le plugin en état zoomé laissait la slide hors cadre, sans retour possible (#42). + */ + _restoreFrame(): void { + const frame = this._slideShadowRoot()?.getElementById('slideViewFrame'); + if (frame) { + frame.style.width = ''; + frame.style.height = ''; + frame.style.transitionDuration = ''; + frame.style.cursor = ''; + frame.style.transform = ''; + } + this.zooming = false; } _addPointer(): void { diff --git a/src/plugins/input/touch/index.ts b/src/plugins/input/touch/index.ts index bf408fb4..ec4c1293 100644 --- a/src/plugins/input/touch/index.ts +++ b/src/plugins/input/touch/index.ts @@ -9,6 +9,11 @@ interface TouchPosition { class TouchInput extends Plugin { touchPosition: { touchstart: TouchPosition; touchend: TouchPosition }; + // Références stables : sans elles, removeEventListener n'a aucune prise et + // `unload()` laisserait les listeners actifs derrière lui (#42). + private readonly onTouchStart = (event: TouchEvent): void => this._captureTouchEvent(event); + private readonly onTouchEnd = (event: TouchEvent): void => this._captureTouchEvent(event, true); + constructor() { super(); this.type = 'inputEvent'; @@ -19,11 +24,17 @@ class TouchInput extends Plugin { } override init(): void { - addEventListener('touchstart', this._captureTouchEvent.bind(this), false); - addEventListener('touchend', e => this._captureTouchEvent(e, true), false); + addEventListener('touchstart', this.onTouchStart, false); + addEventListener('touchend', this.onTouchEnd, false); this.initialized = true; } + override unload(): void { + removeEventListener('touchstart', this.onTouchStart, false); + removeEventListener('touchend', this.onTouchEnd, false); + super.unload(); + } + _captureTouchEvent(event: TouchEvent, forward = false): void { if (!event.view) return; if (contextService.isPresentationIframe(event.view.location.href)) { diff --git a/src/plugins/plugin.ts b/src/plugins/plugin.ts index cf120117..0f2bd2ff 100644 --- a/src/plugins/plugin.ts +++ b/src/plugins/plugin.ts @@ -11,5 +11,18 @@ export class Plugin { } init(): void {} - unload(): void {} + + /** + * Défait ce que `init()` a mis en place. Les surcharges doivent appeler + * `super.unload()` : la classe de base est responsable de l'état qu'elle + * possède, à savoir les callbacks et le drapeau `initialized`. + * + * Sans cette purge, `pluginService.activateOnComponent` empile un callback + * supplémentaire à chaque réactivation — le plugin réémet alors chacun de + * ses événements en autant d'exemplaires qu'il y a eu de cycles (#42). + */ + unload(): void { + this.callbacks = []; + this.initialized = false; + } } diff --git a/test/common/services/plugin.spec.ts b/test/common/services/plugin.spec.ts index 67141651..c02c88da 100644 --- a/test/common/services/plugin.spec.ts +++ b/test/common/services/plugin.spec.ts @@ -1,6 +1,7 @@ import { loadPluginModule } from '@plugins/plugin-loader'; import { logger } from '@services/logger'; import pluginService from '@services/plugin'; +import { Plugin } from '@plugins/plugin'; vi.mock('@plugins/plugin-loader'); @@ -231,5 +232,28 @@ describe('Plugin service', function () { } expect(threw).toBe(false); }); + + // Régression #42 : c'est ici que la fuite se matérialisait. Mesuré sur l'app + // réelle avant correctif, le tableau `callbacks` du singleton grimpait à 1, 2, + // puis 3 sur trois cycles activer/désactiver, et chaque événement du plugin + // partait en autant d'exemplaires. + it('should not accumulate callbacks across activate/unload cycles', async function () { + // Given un vrai Plugin plutôt qu'un mock : le contrat testé est celui de la classe de base + const pluginName = 'pluginName'; + const pluginInstance = new Plugin(); + pluginInstance.type = 'type'; + vi.mocked(loadPluginModule).mockResolvedValue({ instance: pluginInstance }); + const host = { controllerComponentChannel: { broadcast: vi.fn() } }; + + // When trois cycles complets + for (let cycle = 0; cycle < 3; cycle++) { + await pluginService.activateOnComponent(pluginName, host as any); + pluginInstance.unload(); + } + await pluginService.activateOnComponent(pluginName, host as any); + + // Then un seul callback, celui du cycle courant + expect(pluginInstance.callbacks).toHaveLength(1); + }); }); }); diff --git a/test/plugins/input/keyboard/index.spec.ts b/test/plugins/input/keyboard/index.spec.ts index e6a5a6c6..749b4406 100644 --- a/test/plugins/input/keyboard/index.spec.ts +++ b/test/plugins/input/keyboard/index.spec.ts @@ -37,6 +37,38 @@ describe('KeyboardInput', function () { }); }); + describe('unload()', function () { + // La classe de base remet `initialized` à false : sans détacher les listeners, + // une réactivation les enregistrerait une seconde fois (#42). + it('should detach both listeners with the same capture flag used to attach them', function () { + // Given + const removeEventListenerSpy = vi.spyOn(globalThis, 'removeEventListener'); + instance.init(); + + // When + instance.unload(); + + // Then + expect(removeEventListenerSpy).toHaveBeenCalledWith('keyup', expect.any(Function), true); + expect(removeEventListenerSpy).toHaveBeenCalledWith('keydown', expect.any(Function), true); + expect(instance.initialized).toBe(false); + }); + + it('should stop forwarding key events once unloaded', function () { + // Given + instance.init(); + const callback = vi.fn(); + instance.onEvent(callback); + + // When + instance.unload(); + dispatchEvent(new KeyboardEvent('keyup', { code: 'ArrowRight', key: 'ArrowRight' })); + + // Then + expect(callback).not.toHaveBeenCalled(); + }); + }); + describe('_captureKeyboardEvent()', function () { it('should ignore the event when the active element is an ', function () { // Given diff --git a/test/plugins/input/touch-pointer/index.spec.ts b/test/plugins/input/touch-pointer/index.spec.ts index 263913f9..199ec038 100644 --- a/test/plugins/input/touch-pointer/index.spec.ts +++ b/test/plugins/input/touch-pointer/index.spec.ts @@ -86,6 +86,97 @@ describe('TouchPointerInput', function () { expect(document.getElementById('placeholder2')?.innerHTML).toBe(''); expect(instance.initialized).toBe(false); }); + + // Le teardown était incomplet (#42) : le pointeur partait bien du DOM, mais + // callbacks, timer, styles du frame et état de zoom survivaient à la désactivation. + it('should drop the accumulated callbacks on unload', function () { + // Given + instance.init(); + instance.onEvent(() => {}); + + // When + instance.unload(); + + // Then + expect(instance.callbacks).toHaveLength(0); + }); + + it('should clear the pointer hide-timer on unload', function () { + // Given + vi.useFakeTimers(); + const clearIntervalSpy = vi.spyOn(globalThis, 'clearInterval'); + buildCurrentSlide(); + instance.init(); + instance._setPointer('1px', '1px'); + const timer = instance.interval; + expect(timer).toBeDefined(); + + // When + instance.unload(); + + // Then + expect(clearIntervalSpy).toHaveBeenCalledWith(timer); + expect(instance.interval).toBeUndefined(); + vi.useRealTimers(); + }); + + it('should restore the frame styles it had applied on unload', function () { + // Given + const { frame } = buildCurrentSlide(); + instance.init(); + expect(frame.style.cursor).toBe('zoom-in'); + + // When + instance.unload(); + + // Then + expect(frame.style.cursor).toBe(''); + expect(frame.style.width).toBe(''); + expect(frame.style.height).toBe(''); + expect(frame.style.transitionDuration).toBe(''); + }); + + it('should reset a pending zoom on unload, so the slide is never left off-screen', function () { + // Given une désactivation alors que la vue est zoomée + const { frame } = buildCurrentSlide(); + vi.spyOn(frame, 'getBoundingClientRect').mockReturnValue({ width: 800, height: 600 } as DOMRect); + instance.init(); + instance._toggleZoom('0%', '0%'); + expect(instance.zooming).toBe(true); + + // When + instance.unload(); + + // Then + expect(frame.style.transform).toBe(''); + expect(instance.zooming).toBe(false); + }); + + it('should detach the message listener on unload so it can be re-registered later', function () { + // Given + const removeEventListenerSpy = vi.spyOn(globalThis, 'removeEventListener'); + instance.init(); + + // When + instance.unload(); + + // Then + expect(removeEventListenerSpy).toHaveBeenCalledWith('message', expect.any(Function)); + expect(instance.messageEventRegistered).toBe(false); + }); + + it('should stop reacting to messages once unloaded', function () { + // Given + const { shadowRoot } = buildCurrentSlide(); + instance.init(); + instance.unload(); + + // When un message arrive après la désactivation + dispatchEvent(new MessageEvent('message', { data: { type: 'x', data: { type: 'pointerMove', payload: { x: '5%', y: '5%' } } } })); + + // Then rien n'est recréé + expect(shadowRoot.getElementById('pointer')).toBeNull(); + }); }); describe('_addArea() / _removeArea()', function () { diff --git a/test/plugins/input/touch/index.spec.ts b/test/plugins/input/touch/index.spec.ts index 726908ca..6fe273b1 100644 --- a/test/plugins/input/touch/index.spec.ts +++ b/test/plugins/input/touch/index.spec.ts @@ -41,6 +41,25 @@ describe('TouchInput', function () { }); }); + describe('unload()', function () { + // Même contrat que keyboard : la base remet `initialized` à false, donc les + // listeners doivent partir sous peine d'être doublés au réactivage (#42). + it('should detach both listeners with the same capture flag used to attach them', function () { + // Given + const removeEventListenerSpy = vi.spyOn(globalThis, 'removeEventListener'); + instance.init(); + + // When + instance.unload(); + + // Then + expect(removeEventListenerSpy).toHaveBeenCalledWith('touchstart', expect.any(Function), false); + expect(removeEventListenerSpy).toHaveBeenCalledWith('touchend', expect.any(Function), false); + expect(instance.initialized).toBe(false); + vi.restoreAllMocks(); + }); + }); + describe('_captureTouchEvent()', function () { it('should do nothing when the event has no view', function () { // Given diff --git a/test/plugins/plugin.spec.ts b/test/plugins/plugin.spec.ts new file mode 100644 index 00000000..614ac2a3 --- /dev/null +++ b/test/plugins/plugin.spec.ts @@ -0,0 +1,47 @@ +import { Plugin } from '@plugins/plugin'; + +describe('Plugin', function () { + describe('unload()', function () { + // Sans purge, activateOnComponent empile un callback à chaque activation + // (mesuré : 1, 2, 3 sur trois cycles activer/désactiver — issue #42) et chaque + // événement du plugin part alors en autant d'exemplaires. + it('should drop every registered callback', function () { + // Given + const plugin = new Plugin(); + plugin.onEvent(() => {}); + plugin.onEvent(() => {}); + expect(plugin.callbacks).toHaveLength(2); + + // When + plugin.unload(); + + // Then + expect(plugin.callbacks).toHaveLength(0); + }); + + it('should mark the plugin as uninitialized so a later activation re-runs init', function () { + // Given + const plugin = new Plugin(); + plugin.initialized = true; + + // When + plugin.unload(); + + // Then + expect(plugin.initialized).toBe(false); + }); + + it('should be idempotent', function () { + // Given + const plugin = new Plugin(); + plugin.onEvent(() => {}); + + // When / Then + expect(() => { + plugin.unload(); + plugin.unload(); + }).not.toThrow(); + expect(plugin.callbacks).toHaveLength(0); + }); + }); +});