fix: prevent funbox reactivation during page transition to leaderboards (@d1rshan)

[d1rshan]

Bug

bug.mp4

Cause

When navigating from the test page to leaderboards, the leaderboard's sync loading delays setActivePage from firing. During this window, init() checks getActivePage() === "test" which still returns "test" since setActivePage hasn't fired yet, causing funboxes to be incorrectly reactivated.

Fix

Added a new isTestPageVisible signal that is set to false immediately after the test page finishes hiding (before sync loading starts) and back to true after sync loading completes when navigating to test. init() now checks isTestPageVisible() instead of getActivePage() === "test" to determine whether to activate funboxes.

[Leonabcd123]

This actually looks really cool lol, maybe we can add a setting to make funboxes apply globally.

[d1rshan]

yea that would be cool for some effects, i don't know if you watched the vid till the end coz it wouldn't be a good idea for all the funboxes.

[Leonabcd123]

yea that would be cool for some effects, i don't know if you watched the vid till the end coz it wouldn't be a good idea for all the funboxes.

The user could always turn off that setting if they don't want funboxes to apply to the rest of the website. Maybe even have a list of all the funboxes the user wants to have globally applied.

[d1rshan]

The user could always turn off that setting if they don't want funboxes to apply to the rest of the website. Maybe even have a list the user can modify of all the funboxes the user wants to apply globally.

yess that sounds like a good idea :)

i'll see what i can do for "global funboxes" in a new pr coz that doesn't come under the scope of this bug fix,

i'm thinking we can just keep a single toggle control like you said (but without the specific list control for now): apply funboxes globally - yes/no (default will be no) and all the valid funboxes will be applied globally based on this

what do you think?

[Leonabcd123]

i'll see what i can do for "global funboxes" in a new pr coz that doesn't come under the scope of this bug fix,

Let's wait for feedback from miodec or fehmer before starting any work. I'll open a discussion for that (#7874).

i'm thinking we can just keep a single toggle control like you said (but without the specific list control for now): apply funboxes globally - yes/no (default will be no) and all the valid funboxes will be applied globally

what do you think?

Not sure what you mean by "valid" funboxes, I think that global switch should apply to every funbox (although some won't have a visible effect because they're word generation related). Otherwise sounds good!

[d1rshan]

Let's wait for feedback from miodec or fehmer before starting any work. I'll open a discussion for that.

yeah ofc 👍

Not sure what you mean by "valid" funboxes, I think that global switch should apply to every funbox (although some won't have a visible effect because they're word generation related). Otherwise sounds good!

by valid, i mean funboxes that visibly affect the ui without breaking it

[fehmer]

I am pretty sure the funboxes were cleared when leaving the test page at some point. Maybe you can bisect the change which broke this.

I don't think we should add a setting to activate some funboxes on all pages or just test page, very niche use-case.

For the CRT funbox we added special handling to apply on all pages.

[d1rshan]

I am pretty sure the funboxes were cleared when leaving the test page at some point. Maybe you can bisect the change which broke this.

Yes the funboxes are still being cleared, the bug is an edge case where they get reapplied.

NOTE:

• this is repro only when we move away from the test page.
• only funbox effects that affect global css elements are noticable, eg: mirror, space_balls etc and others wouldn't be coz they target selectors that exist in test page only.

---

Issue

The issue is that when leaving the test page, afterHide was still calling TestLogic.restart({ noAnim: true }), and restart() eventually calls init() again.

In this edge case, that allowed Funbox.activate() to run during the route transition and reapply the active funbox CSS right after it had been cleared.

That is why it did not happen on every navigation, but could show up after moving between routes multiple times.

---

Fix

The fix was to keep the existing test page reset, but make that specific teardown restart skip init(), so leaving the test page stays cleanup-only and cannot reactivate funboxes during navigation.

[d1rshan]

I don't think we should add a setting to activate some funboxes on all pages or just test page, very niche use-case.

okay, understood.

For the CRT funbox we added special handling to apply on all pages.

yes, CRT funbox is intentional and it is handling it properly to be applied globally and this PR doesn't affect existing CRT behavior

[fehmer]

This is the commit adding this bug:

765ca95

[d1rshan]

This is the commit adding this bug:

765ca95

Thanks, i was kind of mislead by assuming the bug always existed but went unnoticeable. it looks like 765ca95 exposed the bug.

i found the same timing pattern on:

• frontend/src/ts/pages/account.ts
• frontend/src/ts/pages/friends.ts

just to confirm, do you want me to address the fix close to the regression point or keep my current fix which addresses the teardown directly?

[fehmer]

Can you reproduce the same error when switching from test to account or friends page with funbox mirror active? For me only the switch to the leaderboard does not clear the funbox css.

I think it would be good to find the reason first why this is only broken for the transition to the leaderboard page, than decide how to fix it.

[d1rshan]

ohh yes, weirdly the css is leaking only for leaderboard and other pages are getting the leaked css only after navigating from leaderboard page (this is what happened at the end of the vid)

i'll manually look into it and will try to figure out the exact reason

[d1rshan]

@fehmer, i figured out the exact issue,

so the flow now is:

1. first leaving the test page triggers test restart

2. the restart method eventually reaches init() and this condition is checked:
   

   monkeytype/frontend/src/ts/test/test-logic.ts

   Lines 426 to 428 in 852d158

   	if (getActivePage() === "test") {
   	await Funbox.activate();
   	}

3. (parallely this happens when navigating to leaderboard page)
   leaderboard page follows synchronous loading ie its promise must resolve before setActivePage(nextPage.id) is triggered
   

   monkeytype/frontend/src/ts/controllers/page-controller.ts

   Lines 57 to 65 in 852d158

   	leaderboards: solidPage("leaderboards", {
   	urlParamsSchema: LeaderboardUrlParamsSchema,
   	loadingOptions: {
   	style: "spinner",
   	loadingMode: () => "sync",
   	loadingPromise: async () => {
   	await serverConfigurationPromise;
   	},
   	},

4. getActivePage() inside init() still returns "test" because the active page has not been updated to "leaderboards" yet. so init() assumes the test page is still active and reactivates the funboxes.

and also since leaderboard doesn't bother about cleaning "funbox css" after we move to other pages from leaderboard, the css remains and other pages get affected.

[Copilot]

Pull request overview

Fixes a route-transition issue where exiting the test page could re-run test initialization and re-activate funboxes, leaking funbox global effects (e.g., mirror, space_balls) onto other pages.

Changes:

• Add skipInit option to TestLogic.restart() to allow teardown without calling init().
• Use skipInit: true when the test page hides to prevent funbox reactivation during page transitions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
frontend/src/ts/test/test-logic.ts	Adds skipInit to restart() and conditionally bypasses init() during restart completion.
frontend/src/ts/pages/test.ts	Calls restart({ noAnim: true, skipInit: true }) on afterHide to avoid reinitialization when leaving the test page.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[d1rshan]

hey @fehmer, i updated the fix to a cleaner approach. let me know if you'd like any changes

Fix

Added a new isTestPageVisible signal that is set to false immediately after the test page finishes hiding (before sync loading starts) and back to true after sync loading completes when navigating to test. init() now checks isTestPageVisible() instead of getActivePage() === "test" to determine whether to activate funboxes.