Fix #86679: Prevent document title flicker during navigation - #1
Draft
DragonBot00 wants to merge 1 commit into
Draft
DragonBot00 wants to merge 1 commit into
DragonBot00 wants to merge 1 commit into
Conversation
Three changes: 1. Gate Chrome back-nav workaround behind popstate flag only 2. Coalesce rapid title updates via requestAnimationFrame 3. Skip no-op writes when title unchanged
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposal
Please re-state the problem that we are trying to solve in this issue.
When navigating away from any page (not just expense reports), the browser tab title briefly flashes blank or shows "New Expensify" before settling on the correct destination title. This is caused by
document.title = ''being applied on everyupdateDocumentTitle()call, and multiple triggers firing during a single navigation transition.What is the root cause of that problem?
One line in
src/libs/UnreadIndicatorUpdater/updateUnread/index.tsline 30:This blank write runs inside
setTimeout(0)on every call toupdateDocumentTitle(), not just back-navigation. During a single navigation, 4 independent triggers each schedule their ownsetTimeout(0)with the blank→real sequence:popstatelistener (line 51-53)useDocumentTitlefocus effect (new screen gains focus)The browser paints intermediate frames with the blank title between these macrotasks. Under main-thread load the
setTimeout(0)queue slips, widening the flicker window.Additionally, no deduplication exists — if
updateDocumentTitle()fires 4 times in 50ms, 4 separatesetTimeout(0)callbacks each dodocument.title = ''then set the real title, creating 4 opportunities for the browser to paint a blank tab.What changes do you think we should make in order to solve the problem?
Three surgical changes, all in
src/libs/UnreadIndicatorUpdater/updateUnread/index.ts. Zero API changes —setPageTitleandupdateUnreadkeep their signatures. No new files, no platform splits, no changes touseDocumentTitle.tsorNavigationRoot.tsx.Change 1: Gate the Chrome workaround behind popstate only
The
document.title = ''workaround was added for a Chrome bug wherehistory.go(-1)reverts the title. This only happens on popstate. Normal forward navigation doesn't need it. By gating the blank write, forward navigations get a single clean title write with no intermediate blank.Change 2: Coalesce via requestAnimationFrame instead of setTimeout(0)
All triggers call
scheduleCommit()instead of directly writing. Multiple calls within the same animation frame cancel the previous and schedule one commit. This guarantees at most one title write per frame, eliminating the race between competingsetTimeout(0)callbacks.Change 3: Skip no-op writes
If the computed title already matches
document.title, don't touch it. This prevents redundant DOM writes when the title hasn't actually changed (e.g., unread count update that doesn't affect the title format).Why this is different from the rejected proposals
The C+ reviewer's rejection video showed that prior proposals still had flicker. Here's why those failed and this won't:
"Only clear on popstate" proposals (trasnake87, skylarkerx) — They gated the blank correctly but still used
setTimeout(0). MultiplesetTimeout(0)calls queue as separate macrotasks. Even without the blank, rapid sequential title writes can cause the browser to paint intermediate states if the old title flashes between writes.TaduJR's proposal — Too invasive: rewires title ownership to React Navigation, creates platform-specific file splits (
.web.ts), removessetPageTitleexport. High risk of breaking other screens. The C+ is unlikely to approve a refactor when a surgical fix works.rAF proposal (anonymous) — Good coalescing idea but used
navigationRef.getCurrentOptions()to read the title, coupling the fix to React Navigation internals and requiring a null guard chain.This fix combines the best of all three:
What alternative solutions did you explore? (Optional)