Skip to content

Inherit the editor font in the highlight overlay - #204

Open
Ciinz-04 wants to merge 1 commit into
sysprog21:mainfrom
Ciinz-04:inherit-editor-font
Open

Ciinz-04 wants to merge 1 commit into
sysprog21:mainfrom
Ciinz-04:inherit-editor-font

Conversation

@Ciinz-04

@Ciinz-04 Ciinz-04 commented Oct 1, 2026 •

Copy link
Copy Markdown

Makes the highlight overlay inherit the editor font so the caret stays aligned in Safari on macOS, and adds a browser test that compares the font metrics of the overlay with the textarea's.

Tested on macOS 27.0 with Safari 27.0 (22625.1.29.11.27). The drift was there from the first lines I typed, grew with the line count, and did not go away after starting a new session. After running document.querySelector("#editor-highlight code").style.font = "inherit" in the developer console, the caret stayed aligned with the text on every line.

Refs #82


Summary by cubic

Fixes the highlight overlay caret drifting away from the text on macOS by making the overlay inherit the editor font instead of the user-agent monospace.

  • Adds a browser test that compares the overlay's font metrics with the textarea's across all platforms, not just font family.

Refs #82.

Written for commit 0fb42c3. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

@ColtenOuO ColtenOuO left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A few notes on the description and commit message:

  1. The commit message says "On macOS that is a different font from the textarea's ui-monospace". As far as I know, only Safari resolves ui-monospace; Chrome and Firefox skip it and use a later entry in the stack. That would explain why #82 saw no drift in Chrome on the same Mac, so "Safari on macOS" would be more precise.

  2. In #82, jserv asked whether the drift persists while typing or is temporary and goes away after a new session, as in #155. A font mismatch should show up from the first keystroke and should not go away on its own. Could you say in the description which one you saw? If #155's case isn't explained by this change, Refs #82 may fit better than Closes #82 for now, or note that the temporary case is still open.

Comment thread scripts/browser-check.cjs Outdated
Comment thread scripts/browser-check.cjs
Comment thread scripts/browser-check.cjs Outdated

@Phonlin Phonlin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The CSS fix looks right to me. Agree with jserv's comments that the test should run in CI and compare the full font metrics, and with the point about Closes vs Refs #82 given #155.

The <code> element inside #editor-highlight took font-family: monospace
from the user-agent stylesheet instead of the stack set on the overlay.
Safari on macOS resolves the textarea's ui-monospace to SF Mono, so the
two layers used different fonts, and the taller line box moved the
painted text away from the caret a little more on every line. Other
browsers skip ui-monospace and can land on the same font for both.
@Ciinz-04
Ciinz-04 force-pushed the inherit-editor-font branch from c962454 to 0fb42c3 Compare October 2, 2026 02:16
@jserv
jserv requested a review from ColtenOuO October 2, 2026 02:30
@Ciinz-04

Ciinz-04 commented Oct 2, 2026

Copy link
Copy Markdown
Author

The commit message now says Safari on macOS, since Chrome and Firefox skip ui-monospace.

The drift I saw was there from the first lines I typed, grew with the line count, and did not go away after starting a new session. A font mismatch cannot explain a drift that clears on its own as in #155, so the description now says Refs #82 instead of Closes #82.

The check moved to tests/browser/editor-pipeline.test.js and compares fontFamily, fontSize, lineHeight, fontWeight and letterSpacing of #editor and #editor-highlight code; the browser-check.cjs change is dropped. On main it fails on fontFamily (monospace against the ui-monospace stack), it passes with the fix, and adding a font-size to #editor-highlight code makes it fail again.

One correction: the offline lane does run in CI, as the browser_check_accepts_running_rust_server_offline_interview step in check.yml, though not under scripts/test.sh.

@ColtenOuO ColtenOuO left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants