From 3286477d1e7caf8a4f520aff185ec77868c574c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6?= <14975046+skjnldsv@users.noreply.github.com> Date: Fri, 2 Oct 2026 04:56:04 +0200 Subject: [PATCH] fix(media): rewind instead of reloading once the media has played MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit donePlaying() reloaded the element to bring the poster back, which threw away what it had buffered: every replay downloaded the whole file again. Stopping the player at the start shows plyr's poster just the same. The playground gets a video with a poster of the same name beside it, and an e2e test that the poster is back and a replay fetches nothing. The audio e2e no longer counts the error Firefox raises on a machine with no audio device: the reload used to clear it by accident. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com> --- __tests__/component/media.spec.ts | 64 ++++++++++++++++++ e2e/audio.spec.ts | 5 +- e2e/navigation.spec.ts | 2 + e2e/poster.spec.ts | 58 ++++++++++++++++ lib/composables/usePlyrPlayer.ts | 22 ++++-- playground/App.vue | 3 + .../dav/files/playground/trailer.jpg | Bin 0 -> 5167 bytes .../dav/files/playground/trailer.webm | Bin 0 -> 6603 bytes 8 files changed, 146 insertions(+), 8 deletions(-) create mode 100644 e2e/poster.spec.ts create mode 100644 playground/public/remote.php/dav/files/playground/trailer.jpg create mode 100644 playground/public/remote.php/dav/files/playground/trailer.webm diff --git a/__tests__/component/media.spec.ts b/__tests__/component/media.spec.ts index c040b55..3c8ea3c 100644 --- a/__tests__/component/media.spec.ts +++ b/__tests__/component/media.spec.ts @@ -513,6 +513,70 @@ describe('Videos.vue (smoke)', () => { }) }) +describe('a video that has played to the end', () => { + // What a user puts beside a film to show before and after it plays: + // a picture of the same name in the same folder + async function mountWithPoster() { + const movie = makeFile({ basename: 'trailer.webm', mime: 'video/webm' }) + const poster = makeFile({ basename: 'trailer.jpg', mime: 'image/jpeg' }) + const wrapper = mount(Videos, { props: makeProps({ file: movie, files: [movie, poster] }) }) + await flushPromises() + const video = wrapper.find('video').element as HTMLVideoElement + video.load = vi.fn() + const player = wrapper.findComponent({ name: 'VuePlyrStub' }).vm.player as { stop: Mock } + return { wrapper, video, player, poster } + } + + it('shows the picture of the same name beside it as its poster', async () => { + const { wrapper, poster } = await mountWithPoster() + + expect(wrapper.find('video').attributes('poster')).toBe(poster.encodedSource) + }) + + it('goes back to its poster without downloading the video again', async () => { + const { wrapper, video, player, poster } = await mountWithPoster() + + await wrapper.find('video').trigger('ended') + + // Stopped at the start is what puts plyr's poster back over it, and + // the bytes already buffered stay for the next play + expect(player.stop).toHaveBeenCalledOnce() + expect(video.load).not.toHaveBeenCalled() + expect(wrapper.find('video').attributes('poster')).toBe(poster.encodedSource) + }) + + it('says so, rather than throw, when it has neither a player nor a media element', () => { + const error = vi.spyOn(logger, 'error').mockImplementation(() => {}) + let donePlaying!: () => void + const Host = defineComponent({ + setup() { + const file = makeFile({ basename: 'clip.mp4', mime: 'video/mp4' }) + // No template refs, so neither plyr nor the element ever arrive + donePlaying = usePlyrPlayer(false, makeProps({ file, files: [file] }), (() => {}) as never).donePlaying + return () => h('div') + }, + }) + mount(Host) + + expect(() => donePlaying()).not.toThrow() + expect(error).toHaveBeenCalledWith('Media element not found in donePlaying') + error.mockRestore() + }) + + it('rewinds by itself when there is no player yet', async () => { + const { wrapper, video } = await mountWithPoster() + wrapper.findComponent({ name: 'VuePlyrStub' }).vm.player = undefined + video.pause = vi.fn() + video.currentTime = 12 + + await wrapper.find('video').trigger('ended') + + expect(video.pause).toHaveBeenCalledOnce() + expect(video.currentTime).toBe(0) + expect(video.load).not.toHaveBeenCalled() + }) +}) + describe('media reporting that it plays', () => { it.each([ ['Videos', Videos, 'video', 'clip.mp4', 'video/mp4'], diff --git a/e2e/audio.spec.ts b/e2e/audio.spec.ts index 1267bf4..2b09684 100644 --- a/e2e/audio.spec.ts +++ b/e2e/audio.spec.ts @@ -40,7 +40,10 @@ test.describe('Audio', () => { const state = await audio.evaluate((element: HTMLAudioElement) => ({ readyState: element.readyState, duration: element.duration, - error: element.error?.code ?? null, + // Firefox on a machine with no audio device, as on CI, fails + // to play any sound it has loaded, and says so: that is the + // machine, not the file, and the viewer leaves it be + error: element.error?.message.includes('OnMediaSinkAudioError') ? null : (element.error?.code ?? null), })) expect(state.error).toBeNull() expect(state.readyState).toBeGreaterThan(0) diff --git a/e2e/navigation.spec.ts b/e2e/navigation.spec.ts index d8f90f4..918153f 100644 --- a/e2e/navigation.spec.ts +++ b/e2e/navigation.spec.ts @@ -33,6 +33,8 @@ const MEDIA = [ 'sound.m4a', 'sound.aac', 'clip.webm', + 'trailer.webm', + 'trailer.jpg', ] // No group, so the sheet music only pages among itself diff --git a/e2e/poster.spec.ts b/e2e/poster.spec.ts new file mode 100644 index 0000000..ab72411 --- /dev/null +++ b/e2e/poster.spec.ts @@ -0,0 +1,58 @@ +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { expect, test } from '@playwright/test' +import { ViewerPage } from './support/viewer.ts' + +/** + * A video with a picture of the same name beside it, which people keep + * together to show the picture before the film plays and again once it + * has: `trailer.webm` and `trailer.jpg` in the playground. + */ +test.describe('A video with a poster beside it', () => { + test('shows the poster again once it has played, without downloading the video again', async ({ page }) => { + // Served uncacheable, so that a reload of the element has to fetch + // the file again, as Firefox did on every replay (nextcloud/viewer#2585). + // A rewind plays on from what is already buffered + const requests: string[] = [] + await page.route('**/trailer.webm', async (route) => { + requests.push(route.request().url()) + const response = await route.fetch() + await route.fulfill({ response, headers: { ...response.headers(), 'cache-control': 'no-store' } }) + }) + + const viewer = new ViewerPage(page) + await viewer.open('trailer.webm') + await viewer.waitForOpen() + + const player = viewer.container.locator('.plyr') + const poster = player.locator('.plyr__poster') + await expect(player).toHaveClass(/plyr__poster-enabled/) + await expect(poster).toHaveAttribute('style', /trailer\.jpg/) + + // Played through to the end, muted so no autoplay policy stands in the way + const video = viewer.container.locator('video').first() + await video.evaluate(async (element: HTMLVideoElement) => { + element.muted = true + element.currentTime = 0 + const ended = new Promise((resolve) => element.addEventListener('ended', resolve, { once: true })) + await element.play() + await ended + }) + + // Back at the start and paused, which is what puts plyr's poster on top + await expect(player).toHaveClass(/plyr--stopped/) + await expect(poster).toHaveCSS('opacity', '1') + const downloads = requests.length + + // Playing it again comes from what was already buffered + await video.evaluate(async (element: HTMLVideoElement) => { + const ended = new Promise((resolve) => element.addEventListener('ended', resolve, { once: true })) + await element.play() + await ended + }) + await expect(poster).toHaveCSS('opacity', '1') + expect(requests).toHaveLength(downloads) + }) +}) diff --git a/lib/composables/usePlyrPlayer.ts b/lib/composables/usePlyrPlayer.ts index 5652b45..532fe45 100644 --- a/lib/composables/usePlyrPlayer.ts +++ b/lib/composables/usePlyrPlayer.ts @@ -84,19 +84,27 @@ export function usePlyrPlayer(forAudio: boolean, props: ViewerProps, emit: EmitF } /** - * Reset video after playing to show poster again + * Go back to the start once the media has played, showing its poster again. + * + * Rewound and paused rather than reloaded: plyr shows the poster over a + * player stopped at the start, and what the element already buffered is + * kept. Reloading it brought the poster back too, but every replay then + * downloaded the whole file again (nextcloud/viewer#2585). */ function donePlaying() { - const media = forAudio ? audio : video + if (player.value) { + player.value.stop() + return + } + + const media = (forAudio ? audio : video).value // Should not happen™ - if (!media.value) { + if (!media) { logger.error('Media element not found in donePlaying') return } - - // reset and show poster after play - media.value.autoplay = false - media.value.load() + media.pause() + media.currentTime = 0 } /** diff --git a/playground/App.vue b/playground/App.vue index 78811a6..c3ecf3b 100644 --- a/playground/App.vue +++ b/playground/App.vue @@ -67,6 +67,9 @@ const fixtures: Fixture[] = [ { name: 'sound.m4a', mime: 'audio/mp4' }, { name: 'sound.aac', mime: 'audio/aac' }, { name: 'clip.webm', mime: 'video/webm' }, + // A video with a picture of the same name beside it, its poster + { name: 'trailer.webm', mime: 'video/webm' }, + { name: 'trailer.jpg', mime: 'image/jpeg' }, ] /** Where the fixtures are served from, shaped like a WebDAV path */ diff --git a/playground/public/remote.php/dav/files/playground/trailer.jpg b/playground/public/remote.php/dav/files/playground/trailer.jpg new file mode 100644 index 0000000000000000000000000000000000000000..9f3ee5944cb28489057bebf4b867cbb7bf531003 GIT binary patch literal 5167 zcmex=>ukC3pCfH06P05XITq?4J21E^7eo0A(TN+S4wfI*Oh z@d2X)Gov5_lOQ9rAmjfd4B|kif*gwkfR+Fyqy<3Y%*b*=iHyR93k89a5+F^;QVa}C zj9_hIf@tCl41$XPZ!z#NGXgDT7G$tzczW{dtuL8&jZ}n97p4iYFfurhLhSi4>rjP; zKP%^hg*GDUO)Vb+=5YxhbfTJ&F2_Pv4h=^qE`fli77+z9IN_g#BkB^JKXb{?2>5fT z{h5fp!NQ-d{AV<%E-3!mTl1R8Yp&8(t3z`W{WdI*2wp?EK zaBGyJh-$k0D9FEons9qmHs7Z9Nar;RuLZo;D4k+;N;Z{p0ey*Cf~_2fS`wWM7D@#4 zXt;?eb5Sl3y^x*)YJ&%I?7%f;H2scnfX>)KeF*GQULd2Ews5{JMD&f395hdF5 zRiIDoQGN-Iu$Im2$-Wmu|Qyk zD`f?e!lKJ76RlWY2ruw!q=?#Lw1Lbr5mt`s+bM0B&n5iOsYb)UmGi?wI}!CmEk6RN zCe-a#<3O9ZMNcncJ;|C9B0XGgtjbIi8$AvzNeD8aTp)Cz4oO)AsF<5gb*Ms%a;(qAr*49q+dM%_jzxFH+Aw$tjsYvwh48z`d0<_QQNpeEFn znly2t$Au*sK_;3~BE4L0t;$Z63xo;GQ7~vY#LU4%5hebb3Vlk9eyOg^q75{y2%It~ n83O@UjbihKCQ&ilGS#p?R3vSv$U%VHzax=~Wf7wt^Z%Owi->P> literal 0 HcmV?d00001 diff --git a/playground/public/remote.php/dav/files/playground/trailer.webm b/playground/public/remote.php/dav/files/playground/trailer.webm new file mode 100644 index 0000000000000000000000000000000000000000..4cafdafc25dd63ade99cd620b07cc15726671a08 GIT binary patch literal 6603 zcmcJUg;x~c-^Z5*DFF%T4#}lKU`Y|_PH6$@kd9^PQbM|RVQB;eq=f|u3F$5YL6Gi- z-G^`dp7;ZvduHa$=iHe)@AvDx@60)Ksg+jXQ28hbn*8O%egeUiKZW4Rhei0lv38M< zhM>zwLeQynL+#N3f3>GMB3NVEp%R{~w3HRDFInTN_R+FRLD!pl0P`Co04T12uS|50r>>}(*O(#joyZRTMz(TJr)80gchoV%dg@c;rQT+ zv(i$@18^+C zU#UELBv%X)>Zc#}ld*Tc3bt5yhgiEo+|RP!p;Q-dN?VVIuE52gf3D=swu>o0v7j`( zG0!`aNY>~eGm%X5thwW;1Xuz9{zE0_+k?m6oJphO$6gGAj4uLn>B6TP2s(~N`Q#c% zqSy^PvG~-|jDk#Z+ppSTJ|0r;U#hk;y~E>>aO(BRpyC!E>^_{BAA1&&XKc8ugdKy3^m+MApOjUn@$~%)>QEJkHT_4;hPco3SVG55vca<>!~@lBbaA*3}zr8zZ@5{qb6 z(_VDPxMv|yTWqMzZp;w{As|IjB?HQLz72)S%|O}^kVz2y=rKw81ioLf>s`uQL)|}q zY2=pELSVC_JhR?+&owam%0FSWB|>-kv%@QYqDWPcwG-CiQxIU+rsim%z7=aW>CbRr zsRBAujr#{Lm=yU7!w*67cK^{!ww-1h1wMa~d?WKv#Yn?zJy3j1Ev}fGJ}c?D8L%Kj zDtnZNUzSK zDlU{6)IDaOz4PtzP8!?4`V)@Pc{=Nn?%4|WGC|($iz8i7J?H*(x;9nnY-a$f#p=K* zxn%x+ukULl|4XLET->Y`)tZxOG8nz0d4$l}j9KFrO{@H6Th?bSDSnk%dLJq=QOaf$ z^~Ia>@_seMlAOeT$}Bee8n=J*Zab}!=6xF{*4f(;NH?%m zSM1U$JJvf}gW-^5E*|Yk)CskljG>)`3oJ0$>x(!i3DOZL8};_H&|SDbP6Q^6w`Tr5 zMcI_OsnpYFySUa<+7!RAxmjfIu*WSH)fcFus#LQ}UMfhOR4)Tlu^HH{Sj!h8%for$!edI*$$2tU70yLbE$LbNw%G}m+#H)dD#_x)u zu1UI1gMB{1bK0#Qx4SMRnnm!dI3ijC7>jdyfU332`S7^^vLYOQG_S^lKk zLtXbLT(>BxgOL4H0<)|%~8eC1|fp-`AiIm~GFK5*V=A~X( z9+u_^F9%NQNvK;H8i}!8UpFx8xt_aUsoo=FC=Mw(NScsB@1u8rp`XJnX7D2>)wDE4p7r(luuR6cy`Dy6ew!FPfVyJYcCVXv&N#Ancywa)46wYhff-epD2a$Jmdl)WAG%$GmFQ-fhfm= zD~w=f4Wr`2;H&59=*=~dTKi06?D9G&3bp+UZ186tp=PZQU?dWlVH_qyp^*6;=dK@~ zpY^e6p#yqTAsmi2_GT!%o~|K-6w=Xw`HoY+nxFl6@i*C45DvZpV%u2N($|R|<(p`+ zoQsSh*l}d)9TraPXQVGa?Y5W6MDICSJ5o*yy2NacKQkqeZijIoN|>I5sS(Z~zjx%k z`7aDAwA|L`Viw7KTMx_q)+l)kU{{R8_V>ij;*ag%@h8ucD}JgM;&#*-8jPPdzJ8oB zUo-z%(X$C$;&#nJ_uTla|KU`es2B*tZQXD``j9BBxZyciQV`qJW+2Uj{o)|C+0+u} zxbmX=GFnb&ArF!F6yN(-`!0~(CP}JH%JJy8sdwrXMLW=TTJPCskvKhp+W>xu(Klf1*Zq`8*e&D)5Ns$Cux1*!+XSAp{Lj%SE2_X zc6Wt|wJ%R0>7Ak`ot}5gscZ_Q{MGc$nIi8$hQ8YHmCT;9H0gSkzr7J%(VaeIwWA{! zyBHA5l=_`3E8T=N!|ZXF^e?i#WR|W_(?!jNl)-xK`L_AQxEz-%Q(u?%^C77o6L7zbXrSzqzgeBH%kS*=th8dx90-wkyzf*UXqQrY`X3V;05d{-r0K zoHt2P?-zW(;1Dh)4qAB$OU?=4;AT9Xya|ZCV_3M(@3-4aH%oY&IJi~O7-u20pLcKU zwof(l-NxQGmCYuAAZ4!7X_u9!1B7udiPMXS|K?_xD@PR;eXEYXd6C1`?#jDJ{QX_7 zjExUeYG%#UfNq^Lppf&lwYdD;`d{2H18ekx=2qQ?1G-0XLIR5-)#=$QIZ<+>WCGMBIFqGXL5$RbDX#}z z)S%gMdXDicwp_53OXb??2eJ$)%cv-zrVyK2~G?)z~6i|7ej(%}5r?!;^5eMIIP z9+lr>^sBB1;rLpt7YPYwk-%eVLe;#6gYuW{2dePIv_4Tg>M8&4TF3rZ8p0qVo0H&L zV9ZL&0f0fM&Z6F8yhK;j@Cd7#T~XX*{+t->(F#I(?K31rtxKq4VM0Ca0Y=U@`}7-+ zcidmIwMJRorS9U%-aC@lDdyFLu*=QT)?BR%eloV8xCP(JFh_0MGYyq*@rSbGweu%Y zuVWL_5`BVZBlkFeMBr1Fn_<2ZG~+pW+Zjko!WEt^7r{!?FV1XzSuW$OCh0 z=>I%^nw%>Z86&oKH^(As4`uIrxDh66_rm=r1>;Kesby@7#=%6RFzRxop2`#0B^K;M zyUJr#)Q@&IRy!IukF&Sd_eM=7$pj!USz3mJ<)dJBLabqA`bSfu7SlVrpk_H_p4hM6 z^>Ou()Nk5Vpm5I1(~M?0zbs49Hzu`fSG?7vZ^`j9ub^PP6DLv5o!w9x?yo+Ul#~y=PBx< z?zozofuqkjoCSq{RgFAPWc9xb)akf$C?D24lY87VpUe=!c-x+VobsYl($HyvK6W{C z4RXEi8Bx10$~0JQFOGDNEeeU@L(Cjt1f1(DZ-{s1U43p%EVHHADtjJ5N=6|+*&T=| zVre9GoYL_3r3m_p(YoY%27t>ELIAda5`b*VF&Y~HVC&U(Ta_8t!x7V-v)v5eu>HizGASz$an(`UK+Kb*3}Uf*D&M zTW1cqs7@1#M=!`$24jaY0&}l3|9FHxK{t}|e$5Q;n8jz0oS*tpPc&|;26P%L|H!vp zm(&O~0c17XpXS2p&EioZS9*!oOmt6r+GGg;cJw5_i^sC1SmpI~77M+1HAEORj{?_Y zhPQC%)4{98N)?kzlFpCJh*~<)H-mdazQfGaMYY%qEc%417d{TV{%iuOAZkxVYCV%9 z88$*9!@VjP_SpD$ASD6cxv-3VkKcGuPq<|Xz-2EXfc}4Za038O+&>ik|<5p_X5+=!AoV0qbpA{2NB2RGuXX0bcF?t#GC;d;OO8#eME{)@DjrK|-?M}{{nS47ji ztsLfGW-9KI<={zV+EEF^<}O}Tb9i}{dBu4^g>`e^?<)Sk8|_32eLhVRW>>?;7o&w- zqm-+NG9K~l84da=1;Ow>0{8Ol^M(y2SA)Y{*y=$U@upmXH%&!K=g(5ujve;Xqi)fA zQz~F~7k^w`a~_g_4?*Mk$GIE_06^gWw{J_2rOA-{s4o+fg7=ksA`vp8CV?78at7_r z@@z#gy3qmC>JDaenaI>i-4^y|?W5_b<`#QOwxEe`a<0O|^pAdITV65O61jf7>s)>T z6W*}iFZtbZMK|rOq0Gw6!lFLXMhs$YJ#>)8ptxIplkyFX;CN|K{OvxVyCxE8n@pv- zSg0Dpo39(}E6Hdj_H}9}s86q(QbMxQNOZ!~lkmxj2=W=%$0LWl@IjKNNX%TD)XtYV z84+vD)#0i51*J@T!X#-9dEK0GNMN3?JV9^(+Ub}2lFgC2h;rBa=h$Pfkq{GH_z@9X zO_0yUTzLZzh@aXh^MM8d1TE=r8b`;>q5q)~@r`nH5scVa9;F+#5Z=IVmvSh*`RP$p zF09f*g|BW8G41G7F%p!J0E<1*MVeZE_xPEPY9M8oiO=naW7BfsfBFGtCo}rW*J4}n zHrM2ADDP$f`hn|UbP0WbS!VeM*BNMJL8-H6QWzl=Ottanjh43ihUJ!E-r~HO@}*p> z%QhFPi1;B(#nLa&^on7%5gcjL8|YoH5=_fnXBt~{H~i@{zfAX@iqBjG=I-=XL^_LY z*eYwY-G7ATh#V5hV3u0=<){?5H!jMz)_IfIcLs}z0mkXtQ3v946w2X-+fZ_C#HT^)WM9 zC>62%eM!!_M@7aN0SsR|#q8iaNkY%wH{yM?r9}UZ=HqQqjdv|~WT5oEgNd{{@%_q< zlBYxA=4c|!bG<%OH5fa)~kM~aI{#E7uu?f0%#t=Ro`%kTB9Y3Zf* zTW<+z&37N8$4qvA^=GDzeSR-~f$dyM7&nLyl1Sr8P$!2v=y|ygPJywNP+fQ7CBDCF zt=F5+(EYY45@W3*UNf0gid~=n3xdmOg{?&jIi{xu;?K<7FFIz=-NXawI*p?iC(0s< zU%8%~!pjTG9)hFpXka<>1#QWn2Ync8}dl!EQv&n}xn?_zX=#csbR zqK4G^M6N@JNMfU6Fb3;bt*+@MD4K7E0@&?L>5hvMy5=6$@}p(Xx+`x`JEl1_0o7)( zjOKX}f`BTwkZ=bRs!H*ZKbA!a$4yc$FKeGB$?`|3O)6#P?r9Uv_rb^u#!6OA>jWFM zV;`jt`KyYix-7Wh>^pcg$KBPmTl49C4IAoz{k%bbg+%B*uTUG?pI1*gir?>E8rfrO z7PCYG3a_Y;2m8tm;v2cXgLP8E5`0YLK7XDRxnAx1;=MuVYv&At6kCec_&sx0*n!Vt zLC45I)Taf042GI_HN!-jC{3+R&0Ay>2*qO!$OQi>f`U1IO?4s{vyRXKo9(~qa$!)A zWWHemnUwsQIH#4yUZ~mwM}7zfM6vAIU z#&Zif65t|Fce$8dT9eJv)6d1Wjo)SRz4OD-m*bmSRBf|{kcqi&GCrZ5SRo3p=T4_BhcCZHvI+Nuo4Ckm) z`YrxI>=gtf;@?S14gg4bH4@iS3K>VM8tZ(2_y=#;Yh|w^4vf|LqhwJ2$vvGo(Pb6h zF`aPYm9yS4mk0k`l5t6>vS$hF`FTVcs46ZgELt<|lgT^PIz)vpjUOO<;6OY#yN1Gd zJC|is6&?#eQsWj{9x5NyZ`L7h`*1N3UK7G&HUFdlP;1gdz)sL*Ttxoqxnq;v0p1qv zP_UqRlTQgE*}tHS=1egPTUvyhR1@5bvi3hQM=qTj*zkp|X~-%W9m%xu|J*}d+41~v zSPV!%w81M_X4zpT`&AdaYrM>sg{biOKyMAduigG~0Yxyfor~l_w+T6XE