From b9f6a78fdcd98e76f7f73d15e8c5fcfec41a5b2f Mon Sep 17 00:00:00 2001 From: Simon Date: Sun, 18 Oct 2026 19:40:37 +0000 Subject: [PATCH] Stay on the same video when the phone turns Rotating mid-reel jumped the reel forward a few videos. Two reasons, both about the same arithmetic: every scroll position in the feed is index * scroller.clientHeight, with a spacer standing in for the slides above. A slide was 100dvh, which is not the scroller's height on a phone whose chrome comes and goes, so that mapping addressed the wrong slide by a growing margin; it is now the scroller's own height, which it cannot disagree with. And the realign window lasted two frames. A rotation is many frames and several resizes, with the container's re-anchor on pre-rotation pixels landing in the middle of them -- so the window now stays open until the slide height has held still for a stretch of frames (with a clock to close it in a tab that isn't drawing), and orientationchange and the visual viewport open it too. Moving the feed into a picture-in-picture window settles the same way. tests/smoke_rotate.py replays a rotation as the browser delivers it, on a fixture reel so nothing waits on a real source; against the old code the reel goes from step 7 to step 12. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MPZFnNdHbPGDTqQUNiE4ZN --- frontend/css/style.css | 11 ++- frontend/js/feed.js | 113 +++++++++++++++++----- tests/fixtures/clip.mp4 | Bin 0 -> 25571 bytes tests/smoke_rotate.py | 208 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 305 insertions(+), 27 deletions(-) create mode 100644 tests/fixtures/clip.mp4 create mode 100644 tests/smoke_rotate.py diff --git a/frontend/css/style.css b/frontend/css/style.css index 1e11049..d334ba2 100644 --- a/frontend/css/style.css +++ b/frontend/css/style.css @@ -2246,12 +2246,17 @@ body.feed-mode-open .mode-toggle-btn .icon-svg { /* One screenful. It holds the pane tree rather than a video directly: with several panes, a step shows that many videos at once and one swipe advances - the whole set. */ + the whole set. + + The height is the scroller's own, not 100dvh: every scroll position in the + feed is computed as `index * scroller.clientHeight` (the top spacer stands + in for the slides above), so a slide that is even a few pixels off that + height makes the mapping drift by a slide's worth after a handful of them -- + which is what used to make rotating the phone land on a different video. */ .feed-slide { position: relative; width: 100%; - height: 100vh; - height: 100dvh; + height: 100%; scroll-snap-align: start; scroll-snap-stop: always; overflow: hidden; diff --git a/frontend/js/feed.js b/frontend/js/feed.js index 3d2cf8b..4cd0c31 100644 --- a/frontend/js/feed.js +++ b/frontend/js/feed.js @@ -47,6 +47,19 @@ App.feed = App.feed || {}; // slide is playing. Normal swipes (no resize in flight) are unaffected. let suppressScroll = false; let resizeSettleRaf = null; + let resizeSettleTimer = null; + // A rotation is not one event. The viewport, the browser's own chrome and + // the scroll-snap container settle over many frames and often several + // `resize` events, so the realign window is held open until the slide + // height has held still for a stretch of frames -- not for a fixed two, + // which ran out while the viewport was still moving and let the stale + // scroll positions through, skipping the reel forward a few videos. + const SETTLE_STABLE_FRAMES = 10; + const SETTLE_MAX_MS = 1500; + let settleStartedAt = 0; + let settleStableFrames = 0; + let settleHeight = -1; + let settleDone = null; // run once the window closes, if anything asked // HUD auto-hide: the reels HUD fades out after this much inactivity and // reappears on any pointer movement / tap / scroll. Buttons keep their @@ -1313,23 +1326,63 @@ App.feed = App.feed || {}; if (activeSlide) measureFeedTitle(activeSlide); }; + // Ends the realign window: one last realign, then real swipes count again. + const finishSettle = function() { + if (resizeSettleRaf) { + cancelAnimationFrame(resizeSettleRaf); + resizeSettleRaf = null; + } + if (resizeSettleTimer) { + clearTimeout(resizeSettleTimer); + resizeSettleTimer = null; + } + realignToActive(); + suppressScroll = false; + const after = settleDone; + settleDone = null; + if (after) after(); + }; + + // Watches the viewport settle, frame by frame, holding the active video in + // place the whole way. + const settleStep = function() { + resizeSettleRaf = null; + if (!state.feedOpen || state.feedActiveIndex < 0) { + suppressScroll = false; + return; + } + const height = slideHeight(); + if (height === settleHeight) settleStableFrames += 1; + else { + settleHeight = height; + settleStableFrames = 0; + } + realignToActive(); + if (settleStableFrames < SETTLE_STABLE_FRAMES && + (Date.now() - settleStartedAt) < SETTLE_MAX_MS) { + resizeSettleRaf = requestAnimationFrame(settleStep); + return; + } + // One more frame, so a re-snap the container has already scheduled + // lands inside the window rather than just after it. + resizeSettleRaf = requestAnimationFrame(finishSettle); + }; + const onResize = function() { if (!state.feedOpen || state.feedActiveIndex < 0) return; // Suppress scroll handling while we realign so the container's re-snap // doesn't flip the active video, then re-enable it once layout settles. suppressScroll = true; + settleStartedAt = Date.now(); + settleStableFrames = 0; + settleHeight = -1; realignToActive(); - // Orientation changes can settle over more than one frame (the visual - // viewport and the scroll-snap re-anchor in stages); realign again once - // layout has settled, then stop suppressing real swipes. if (resizeSettleRaf) cancelAnimationFrame(resizeSettleRaf); - resizeSettleRaf = requestAnimationFrame(() => { - realignToActive(); - resizeSettleRaf = requestAnimationFrame(() => { - resizeSettleRaf = null; - suppressScroll = false; - }); - }); + resizeSettleRaf = requestAnimationFrame(settleStep); + // Frames stop in a hidden tab, which would leave the window open for + // as long as the tab stays away; the clock closes it regardless. + if (resizeSettleTimer) clearTimeout(resizeSettleTimer); + resizeSettleTimer = setTimeout(finishSettle, SETTLE_MAX_MS + 500); }; // ------------------------------------------------------------------ @@ -1559,21 +1612,21 @@ App.feed = App.feed || {}; // recomputes them from the active video's id -- the same thing an // orientation change does. const settleAfterMove = function() { - suppressScroll = true; - realignToActive(); - requestAnimationFrame(() => { - realignToActive(); - requestAnimationFrame(() => { - suppressScroll = false; - const slide = slidesByIndex.get(state.feedActiveIndex); - if (slide) panesOf(slide).forEach((pane) => { - const video = pane.querySelector('.feed-video'); - if (video && video.paused && pane.classList.contains('is-loaded')) { - video.play().catch(() => {}); - } - }); + const resume = function() { + const slide = slidesByIndex.get(state.feedActiveIndex); + if (slide) panesOf(slide).forEach((pane) => { + const video = pane.querySelector('.feed-video'); + if (video && video.paused && pane.classList.contains('is-loaded')) { + video.play().catch(() => {}); + } }); - }); + }; + if (!state.feedOpen || state.feedActiveIndex < 0) { + resume(); + return; + } + settleDone = resume; + onResize(); }; // Puts the feed back in the page. Called when the window closes, however it @@ -1720,6 +1773,10 @@ App.feed = App.feed || {}; cancelAnimationFrame(resizeSettleRaf); resizeSettleRaf = null; } + if (resizeSettleTimer) { + clearTimeout(resizeSettleTimer); + resizeSettleTimer = null; + } const spacer = getTopSpacer(); if (spacer) spacer.style.height = '0px'; const scroller = getScroller(); @@ -1749,6 +1806,14 @@ App.feed = App.feed || {}; if (!scrollBound) { scroller.addEventListener('scroll', onScroll, { passive: true }); window.addEventListener('resize', onResize); + // `orientationchange` fires before the new metrics are readable and + // the visual viewport moves on its own as the browser's chrome + // slides away, so both open the realign window too -- the watcher + // below waits for the height to stop moving either way. + window.addEventListener('orientationchange', onResize); + if (window.visualViewport) { + window.visualViewport.addEventListener('resize', onResize); + } scrollBound = true; } diff --git a/tests/fixtures/clip.mp4 b/tests/fixtures/clip.mp4 new file mode 100644 index 0000000000000000000000000000000000000000..1c60270e55d3894cafe7f11ef78c1a396059ec77 GIT binary patch literal 25571 zcmeHOZ)_aJ6`!*c1T>I@+To9)YC{cDEBm~A=Q@tjsy;`7M4|{45>y50y0^RMTfBR_ z-d)>gzLcUgid6aqq)17SuToWol<@BZMMzWuRH5=ueCQWaA(b{#0fG1<5h)1p@!rnd z?cUzW99BXhdaJve*_k)LeLM5so0&K77-Ld7T=J~Ibr|bon8G)wE$E8ljVX-XxZiQz z1;&`|EL03g6IcJcjLpj|!|)SdZ8uPyx}SAyWwEyYqL;!cQ_k8vp+#7(BQf0 zEA^ai&&^E1TTVm`hfE_3Ah*@DgHS*L0^+U{JP)lnhNVHB$V!ci8Od&{f2qpmUw>1 z3c`{N`4@sPXbuR{JcWY>DPBoTUN8l(y&JOl#Va#-E6|TCS!G97krm0dO10eh7_W?e zuW2+O-40YcGX@)M{D)mP1L?`rg{eKCd7ByQ$H(-u+{P^;P>AVo&-J5UOJ(aW3m!#!Tt1YtA+m@eBkZ^aH@Xgg2%7xy}w=5g-BAUVId<($Ts#updnAP2Ng z=ly`)LF?3uXPp|!HTvT9Xk??Yjm8)J^qBtOHK@B+)Ioy&`~D4(8GtVw54675LH>ty zPyHcefaKx(tq<42IN})|>G}7297zvp{8pQnNzeHjhV+E=WHmkcuE_`LZIa(!?gxTU zccC3#u(rSP_mFDyMyNshE60pdtlO51bb{z8U-LW+MrvKd77wQyIuW`ExYf|2#sJC~ zgdCM2@Ql6A0t*Y5efvL*1{G?n{l)U7aYQ?Nc!F&mF+a)hpf~N3W^bJ+X%FqaY zB*&_mMiCJeXi@aF>Wrx>w;U}*MyzUR`nCo{P+qd`oZSwG zH30X@uE8|ido4Jm0e0U{!+qQd$GNLt*#TD{X~A*s>Z2*RV`H6goV)rjJK^S9aGbk( zkcO*u!U^sUC*VZur(1A>yJuQ(Sg@sV?72=j!QHnK`f#%sGG|!?|)MZ*t5b@aaUtyVhZQD%^!npzso#yKU?>km3Pj3ak=w&V{Yt^`qPg& zi$Bf&p*BBv=#GD6A9(4OvuD}*kFGjd-1qO>Hr}wZ9*(f5@86c+|C0w_|L==?9*L&Q zc4r(rg@gRfy>ZwI{9Um(?mj5!?v2Bmuk*$s4Zy|TIK*GOH%@RDd*cLLZ{9f0Ou`!{ z3Sw^@hwITBCtAm=HO^ha8z7)= zrK}h79m!pX+cx&bAq~l0r(+bm8P<}kdVzW0)k*G1?%GZtgj|rlJ0H7KvUf1`^mgh+ zatAZn`8xF?xg)u2`dO2wtwJs?*l8=t9myTZ9jv~=mdB@FLMv!L+jXg3PezxBRRHa0 z+g6`zjA|meTg@AHfNgG^Z*LrmZ`vCdjY2Xuj>~u=MyX@g#DZ_d8;1`9Crk1yL2wsa zFHz8o^+M9GJHz5|iC7W-vfXwGoV&K&DHgSzor;29V&epNNw0`#9mmEA?qaV97Qmd| zB{oiQm$Y62ZcWyUbJrFd$Kh7<#&Pbtot<*-+G69780MuoCR5 { + localStorage.setItem('config', JSON.stringify({ servers: [{ [server]: {} }] })); + localStorage.setItem('preferences', JSON.stringify({ [server]: { channelId: channel } })); + localStorage.removeItem('session'); +}""" + +WHERE = """() => { + const scroller = document.getElementById('feed-scroll'); + const active = document.querySelector('.feed-slide.is-active'); + const h = scroller.clientHeight; + return { + index: App.state.feedActiveIndex, + videoId: App.state.feedActiveVideoId, + scrollTop: scroller.scrollTop, + expected: App.state.feedActiveIndex * h, + height: h, + // Where the active slide actually sits: the one thing the reader sees. + activeTop: active ? Math.round(active.getBoundingClientRect().top) : null, + activeId: active ? active.dataset.step : null, + // Every rendered slide must be exactly a scroller tall, or + // `index * clientHeight` stops addressing the slide it names. + offHeight: Array.from(document.querySelectorAll('.feed-slide')) + .map((s) => s.getBoundingClientRect().height) + .filter((sh) => Math.abs(sh - h) > 1) + }; +}""" + +# A rotation as the browser actually delivers it: another resize while things +# are still moving, then the container's own re-anchor on a position measured +# against the height from before. +ROTATION_NOISE = """(oldHeight) => new Promise((resolve) => { + const scroller = document.getElementById('feed-scroll'); + const index = App.state.feedActiveIndex; + setTimeout(() => window.dispatchEvent(new Event('resize')), 60); + setTimeout(() => { + scroller.scrollTop = index * oldHeight; + resolve(index * oldHeight); + }, 140); +})""" + +GO = """(step) => { + const scroller = document.getElementById('feed-scroll'); + scroller.scrollTop = step * scroller.clientHeight; + return App.state.feedActiveIndex; +}""" + + +class Checks: + def __init__(self): + self.failed = 0 + + def ok(self, label, condition, detail=""): + mark = "PASS" if condition else "FAIL" + if not condition: + self.failed += 1 + print(f" [{mark}] {label}" + (f" -- {detail}" if detail and not condition else "")) + + +def swipe_to(page, step): + """One step at a time, the way a thumb gets there: only the slides around + the active one exist, so the scroll range doesn't reach step 7 in one go.""" + for n in range(1, step + 1): + page.evaluate(GO, n) + page.wait_for_timeout(250) + page.wait_for_timeout(400) + + +def items(): + return [{"id": f"vid-{i}", "title": f"clip {i}", + "url": f"https://fixture.test/v{i}.mp4", "thumb": "", "channel": CHANNEL, + "duration": 30, "isLive": False, "tags": []} for i in range(36)] + + +def rotate(page, c, size, label): + before = page.evaluate(WHERE) + page.set_viewport_size(size) + stale = page.evaluate(ROTATION_NOISE, before["height"]) + page.wait_for_timeout(1400) + after = page.evaluate(WHERE) + print(f" (was step {before['index']} at {before['scrollTop']}px of {before['height']};" + f" the stale re-anchor asked for {stale}px)") + c.ok(f"{label}: the same video is still the one playing", + after["videoId"] == before["videoId"], f"{before['videoId']} -> {after['videoId']}") + c.ok(f"{label}: and the same step", + after["index"] == before["index"], f"{before['index']} -> {after['index']}") + c.ok(f"{label}: the scroll position is the one that step sits at", + abs(after["scrollTop"] - after["expected"]) <= 1, str(after)) + c.ok(f"{label}: so the step fills the screen rather than straddling two", + after["activeTop"] == 0, str(after["activeTop"])) + c.ok(f"{label}: every slide is exactly a screenful", + after["offHeight"] == [], str(after["offHeight"])) + return after + + +def main(): + c = Checks() + + with sync_playwright() as p: + browser = p.chromium.launch(args=[ + "--no-sandbox", "--disable-dev-shm-usage", "--disable-gpu", + "--autoplay-policy=no-user-gesture-required", + "--renderer-process-limit=1", "--js-flags=--max-old-space-size=512", + ]) + page = browser.new_page(viewport=PORTRAIT) + crashes = [] + page.on("pageerror", lambda e: crashes.append(str(e))) + + page.route("**/api/status", lambda route: route.fulfill( + status=200, content_type="application/json", body=json.dumps(STATUS))) + page.route("**/api/videos", lambda route: route.fulfill( + status=200, content_type="application/json", + body=json.dumps({"items": items(), "pageInfo": {"hasNextPage": False}}))) + page.route(re.compile(r"\.mp4"), lambda route: route.fulfill( + status=200, content_type="video/mp4", body=open(CLIP, "rb").read())) + + page.goto(BASE, wait_until="domcontentloaded") + page.evaluate(SEED, [SERVER, CHANNEL]) + page.goto(BASE, wait_until="load") + page.wait_for_selector(".video-card", timeout=30000) + page.wait_for_timeout(500) + page.evaluate("() => App.feed.open()") + page.wait_for_selector(".feed-slide", timeout=20000) + page.wait_for_timeout(800) + + print("\nseven videos into the reel") + swipe_to(page, 7) + start = page.evaluate(WHERE) + c.ok("that is where the reel is", start["index"] == 7, str(start)) + c.ok("and the slides are a screenful each to begin with", + start["offHeight"] == [], str(start["offHeight"])) + + print("\nturning it sideways") + land = rotate(page, c, LANDSCAPE, "landscape") + c.ok("the slides took the new height", land["height"] == 430, str(land["height"])) + + print("\nand back upright") + port = rotate(page, c, PORTRAIT, "portrait") + c.ok("the slides took the height back", port["height"] == 930, str(port["height"])) + + print("\nswiping still works once it has settled") + page.evaluate(GO, port["index"] + 1) + page.wait_for_timeout(700) + moved = page.evaluate(WHERE) + c.ok("a real swipe after a rotation moves the reel on", + moved["index"] == port["index"] + 1, str(moved["index"])) + + print("\nrotating twice in a row, as a wrist does") + page.set_viewport_size(LANDSCAPE) + page.wait_for_timeout(90) + page.set_viewport_size(PORTRAIT) + page.wait_for_timeout(1500) + twice = page.evaluate(WHERE) + c.ok("the reel is where it was left", twice["index"] == moved["index"] and + twice["videoId"] == moved["videoId"], str(twice)) + c.ok("and sitting on its snap point", + abs(twice["scrollTop"] - twice["expected"]) <= 1, str(twice)) + + c.ok("nothing threw along the way", not crashes, str(crashes[:2])) + browser.close() + + print(f"\n{'OK' if not c.failed else 'FAILED'}: {c.failed} check(s) failed") + return 1 if c.failed else 0 + + +sys.exit(main())