Skip to content

fix: navbar font mismatch and overlap before Bootstrap loads - #1039

Merged
boomzero merged 5 commits into
devfrom
codex/fix-navbar-cjk-font
Oct 4, 2026
Merged

boomzero merged 5 commits into
devfrom
codex/fix-navbar-cjk-font

Conversation

@boomzero

@boomzero boomzero commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

What does this PR aim to accomplish?:

After #1036, the monochrome navbar still renders Chinese text in different fonts on /web and classic pages. Both pages load Source Serif 4 and have identical computed font-family, but that font contains no Chinese glyphs. Classic declares lang=en while /web declares lang=zh-CN. On macOS Chromium, the browser picks PingFang SC for classic and Songti SC for /web. Switching the live /web document language to en also changes its rendered Chinese navbar font to PingFang SC.

A second reproducible failure affects generic page layouts when the Bootstrap resource cache is empty and its CDN fallback is delayed or fails. NavbarStyler sets position:fixed but relies on Bootstrap's .fixed-top for top:0. With top:auto, inserting the body spacer pushes the navbar down with the page, where it covers the first content instead of anchoring to the viewport. An offline layout probe reproduces overlap in all 24 width/skin/theme/menu combinations per version on both stable and pre-fix dev.

Related to issue #1038. Its reported all-page overlap remains unconfirmed in the reporter's browser because it contains no reproduction steps. This PR fixes the demonstrated missing-CSS condition and does not claim that it is the confirmed cause of that report.

How does this PR accomplish the above?:

Give the monochrome navbar and its links one explicit font stack, preserving the existing Latin serif fonts and adding PingFang SC, Microsoft YaHei and Noto Sans CJK SC before the generic fallback. The brand, user dropdown and dropdown items inherit that navbar stack. Body typography remains unchanged.

Pin the navbar top offset directly in NavbarStyler, so viewport anchoring works before Bootstrap is available. Preserve and restore its original inline top value and priority during teardown.

Validation:

  • The new offline Chromium regression compares actual rendered glyph fonts through CSS.getPlatformFontsForNode for the brand, navigation links, username span and dropdown items under both document languages. It fails on the pre-fix code (Songti SC versus PingFang SC) and passes with this change.
  • The regression bundles the unmodified 20 KB Latin subset of the existing Source Serif 4 font with its OFL license. An unloaded font or local substitute does not reproduce the bug; the test asserts the webfont loaded. No new dependency was added.
  • A new browser regression fails before the positioning fix and passes after it, checking mobile/desktop widths and both skins without Bootstrap, after its positioning CSS arrives, and after teardown. It also checks restoration of an existing important top offset.
  • The wider offline layout probe has zero overlaps with the fixed code in all 24 combinations, both with and without Bootstrap. Stable code remains the failing control without Bootstrap.
  • All 20 npm tests pass using the installed Chromium. Userscript syntax and diff whitespace checks pass.
  • A separate automated probe injects the shared initialization and actual navbar conversion code into the public /web/contest and classic home pages. Both now render Chinese navbar glyphs with PingFang SC, Latin glyphs with Source Serif 4, and matching font size, weight and spacing. This is not an end-to-end test of an installed userscript or authenticated account.

Please verify this with the installed userscript in both UIs before merging. The final checklist is unchecked because template item 11 requires human verification; the earlier PR was merged without that check.


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributor's guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented on my proposed changes within the code.
  3. I have tested my changes.
  4. I am willing to help maintain this change if there are issues with it later.
  5. It is compatible with the GNU General Public License v3.0
  6. I have squashed any insignificant commits. (git rebase)
  7. I have checked that another pull request for this purpose does not exist.
  8. I have considered and confirmed that this submission will be valuable to others.
  9. I accept that this submission may not be used, and the pull request can be closed at the will of the maintainer.
  10. I give this submission freely and claim no ownership to its content.
  11. I have verified that my changes work correctly in both the new UI and the old/classic UI.

  • I have read the above and my PR is ready for review. Check this box to confirm

Summary by Sourcery

Fix monochrome navbar font consistency and prevent content overlap while Bootstrap styles are unavailable.

Bug Fixes:

  • Standardize Chinese glyph rendering across legacy and web monochrome navigation bars by adding an explicit shared font fallback stack.
  • Prevent the navigation bar from overlapping page content when Bootstrap styles are unavailable or delayed by enforcing viewport top anchoring.

Enhancements:

  • Preserve and restore existing inline navigation top offsets, including priority, when navigation styling is removed.

Build:

  • Bump the userscript and package versions to 3.8.2.

Tests:

  • Add browser regressions covering language-independent navbar glyph fonts and layout behavior before and after Bootstrap loading.

Chores:

  • Add the Source Serif 4 fixture and its licensing information for offline font rendering tests.
  • Record the 3.8.2 update metadata.

@sourcery-ai

sourcery-ai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Version 3.8.2 fixes inconsistent Chinese glyph fallback in the monochrome navbar by applying an explicit shared font stack, and prevents content overlap when Bootstrap is delayed or unavailable by setting and restoring the navbar’s top offset. New offline Chromium regressions cover rendered fonts, responsive pre-Bootstrap positioning, teardown, and inline-style preservation; human verification in both new and classic UIs remains required.

Sequence diagram for pre-Bootstrap navbar positioning

sequenceDiagram
    participant Page
    participant NavbarStyler
    participant Navbar
    participant Bootstrap

    Page->>NavbarStyler: apply()
    NavbarStyler->>Navbar: preserveStyles(top)
    NavbarStyler->>Navbar: classList.add(fixed-top)
    NavbarStyler->>Navbar: style.top = 0
    Note over Navbar,Bootstrap: Navbar stays anchored even when Bootstrap CSS is delayed or unavailable
    Bootstrap-->>Navbar: load fixed-top CSS
    Page->>NavbarStyler: remove()
    NavbarStyler->>Navbar: restore original top style
Loading

Flow diagram for consistent monochrome navbar fonts

flowchart LR
    A[Document language: en or zh-CN] --> B[Monochrome navbar]
    B --> C[Explicit shared navbar font stack]
    C --> D[Chinese glyphs use consistent CJK fallback]
    C --> E[Latin glyphs retain Source Serif 4]
Loading

File-Level Changes

Change Details Files
Normalize monochrome navbar typography across document languages with a shared fallback stack for Chinese glyphs.
  • Add a navbar-specific font variable retaining Source Serif 4 for Latin text and adding common Chinese fonts.
  • Apply the stack to the navbar and navigation links so brand, user, and dropdown content inherit consistent rendering while body typography stays unchanged.
XMOJ.user.js
Anchor the navbar explicitly before Bootstrap CSS is available and make teardown preserve existing inline styles.
  • Set the navbar’s inline top offset to 0 alongside the existing positioning behavior.
  • Preserve and restore the original top value and CSS priority during destruction.
XMOJ.user.js
Add browser regressions for font fallback and pre-Bootstrap navbar layout behavior.
  • Compare platform-rendered glyph fonts under en and zh-CN documents using an embedded Source Serif fixture.
  • Test mobile and desktop layouts with and without Bootstrap, including content spacing, CSS arrival, teardown, and important inline top restoration.
  • Document and license the offline font fixture used to ensure the webfont is actually loaded.
tests/navbar-styler.test.cjs
tests/fixtures/fonts/README.md
tests/fixtures/fonts/OFL.txt
Release the fix as version 3.8.2 and record the update metadata.
  • Bump userscript and package versions.
  • Add release/update information for the navbar fixes.
XMOJ.user.js
package.json
Update.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@hendragon-bot hendragon-bot Bot added the user-script This issue or pull request is related to the main user script label Oct 3, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Deploying xmoj-script-dev-channel with  Cloudflare Pages  Cloudflare Pages

Latest commit: c730563
Status: ✅  Deploy successful!
Preview URL: https://baf39141.xmoj-script-dev-channel.pages.dev
Branch Preview URL: https://codex-fix-navbar-cjk-font.xmoj-script-dev-channel.pages.dev

View logs

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/navbar-styler.test.cjs" line_range="192" />
<code_context>
+
+test('Chinese navbar glyphs use the same fonts under legacy and web document languages', {timeout: 60000}, async () => {
+    const skin = source.match(/const MonochromeSkinCSS = `([\s\S]*?)`;/)[1];
+    const font = fs.readFileSync(path.join(__dirname, "fixtures/fonts/source-serif-4-latin.woff2")).toString("base64");
+    const browser = await chromium.launch({executablePath: process.env.XMOJ_CHROMIUM || undefined});
+    try {
</code_context>
<issue_to_address>
**issue (testing):** The navbar regression test unconditionally reads `tests/fixtures/fonts/source-serif-4-latin.woff2`, but that fixture is absent from the diff and the fixtures directory contains only `OFL.txt` and `README.md`; running the test therefore raises `ENOENT` before launching Chromium.

**Triggers:** When `npm test` or `tests/navbar-styler.test.cjs` runs.

**Suggested fix:** Add the referenced WOFF2 fixture at the expected path, or update the test to reference a file that is actually included.
</issue_to_address>

Sourcery assessment

Approval pending. 1 finding to address first.

Blocking findings: tests/navbar-styler.test.cjs:192


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread tests/navbar-styler.test.cjs
sourcery-ai[bot]
sourcery-ai Bot previously approved these changes Oct 3, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery assessment

Approved.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Re-trigger cubic

@boomzero boomzero changed the title fix: match Chinese navbar fonts between web and classic UI fix: navbar font mismatch and overlap before Bootstrap loads Oct 4, 2026
@github-actions
github-actions Bot force-pushed the codex/fix-navbar-cjk-font branch from 1011a13 to c730563 Compare October 4, 2026 00:59
@sourcery-ai
sourcery-ai Bot dismissed their stale review October 4, 2026 00:59

Sourcery withdrew this approval because the latest commits introduced blocking findings.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/navbar-styler.test.cjs">

<violation number="1" location="tests/navbar-styler.test.cjs:250">
P3: This late-CSS check is vacuous: inline `position: fixed` and `top: 0` already match the injected rule, while the fixed height leaves the asserted vertical geometry unchanged. Assert a Bootstrap-only effect, such as the navbar's horizontal bounds, so this check detects a broken late stylesheet transition.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

assert.equal(before.navbarTop, mono ? 0 : 16, 'the navbar must anchor to the viewport without Bootstrap');
assert.ok(before.contentTop >= before.navbarBottom, 'the navbar must not cover content while CSS is unavailable');
// When Bootstrap finally arrives, the reserved space still works.
await page.addStyleTag({content: '.fixed-top { position: fixed; top: 0; right: 0; left: 0; }'});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This late-CSS check is vacuous: inline position: fixed and top: 0 already match the injected rule, while the fixed height leaves the asserted vertical geometry unchanged. Assert a Bootstrap-only effect, such as the navbar's horizontal bounds, so this check detects a broken late stylesheet transition.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tests/navbar-styler.test.cjs, line 250:

<comment>This late-CSS check is vacuous: inline `position: fixed` and `top: 0` already match the injected rule, while the fixed height leaves the asserted vertical geometry unchanged. Assert a Bootstrap-only effect, such as the navbar's horizontal bounds, so this check detects a broken late stylesheet transition.</comment>

<file context>
@@ -223,3 +223,44 @@ test('Chinese navbar glyphs use the same fonts under legacy and web document lan
+                assert.equal(before.navbarTop, mono ? 0 : 16, 'the navbar must anchor to the viewport without Bootstrap');
+                assert.ok(before.contentTop >= before.navbarBottom, 'the navbar must not cover content while CSS is unavailable');
+                // When Bootstrap finally arrives, the reserved space still works.
+                await page.addStyleTag({content: '.fixed-top { position: fixed; top: 0; right: 0; left: 0; }'});
+                const after = await Measure();
+                assert.ok(after.contentTop >= after.navbarBottom);
</file context>

@boomzero boomzero mentioned this pull request Oct 4, 2026
2 tasks done
@zsTree0830

Copy link
Copy Markdown
Member

这一版的 topbar 字体都变了

@zsTree0830

Copy link
Copy Markdown
Member

奇怪
换一个浏览器配置就好了

@zsTree0830 zsTree0830 closed this Oct 4, 2026
@boomzero boomzero reopened this Oct 4, 2026
@boomzero

boomzero commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

这一版的 topbar 字体都变了

? Intended.

@boomzero

boomzero commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

但是这个PR 修的是别的问题

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="XMOJ.user.js" line_range="2891" />
<code_context>
             n.classList.add('fixed-top', 'container', 'ml-auto');
+            // The CDN fallback may still be loading when the top bar starts. Without
+            // Bootstrap's .fixed-top rule, top:auto moves it down with the spacer.
+            n.style.top = '0';
             if (UtilityEnabled("MonochromeUI")) {
                 Object.assign(n.style, {
</code_context>
<issue_to_address>
**issue (bug_risk):** An existing inline `top: ... !important` declaration prevents `n.style.top = '0'` from taking effect, so the navbar keeps its page-provided offset instead of being pinned to the viewport top. With a sufficiently large important offset, the navbar still overlaps the page content despite the positioning fix.

**Triggers:** When the target navbar already has an inline `top` declaration with `!important`.

**Suggested fix:** Set the temporary offset with `n.style.setProperty('top', '0', 'important')`, while retaining the captured value and priority for teardown.
</issue_to_address>

Sourcery assessment

Approval pending. 1 finding to address first.

Blocking findings: XMOJ.user.js:2891


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread XMOJ.user.js
n.classList.add('fixed-top', 'container', 'ml-auto');
// The CDN fallback may still be loading when the top bar starts. Without
// Bootstrap's .fixed-top rule, top:auto moves it down with the spacer.
n.style.top = '0';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): An existing inline top: ... !important declaration prevents n.style.top = '0' from taking effect, so the navbar keeps its page-provided offset instead of being pinned to the viewport top. With a sufficiently large important offset, the navbar still overlaps the page content despite the positioning fix.

Triggers: When the target navbar already has an inline top declaration with !important.

Suggested fix: Set the temporary offset with n.style.setProperty('top', '0', 'important'), while retaining the captured value and priority for teardown.

@boomzero
boomzero merged commit 11dd0db into dev Oct 4, 2026
15 checks passed
@boomzero
boomzero deleted the codex/fix-navbar-cjk-font branch October 4, 2026 05:07
@zsTree0830 zsTree0830 linked an issue Oct 5, 2026 that may be closed by this pull request
2 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L user-script This issue or pull request is related to the main user script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 内容被顶栏覆盖

2 participants