Guard against Infinity height in HeightMap which results in NaN to value and offset #72

Closed
sfc-gh-klin wants to merge 1 commit from infinity-nan-height into main
sfc-gh-klin commented 2025-07-03 23:50:08 +02:00 (Migrated from github.com)

I was previously on codemirror/view 6.17.1 (Aug 31, 2023) and upgraded to codemirror/view 6.36.3 (Feb 17, 2025).
Likewise upgraded from codemirror/state 6.4.1 (Feb 19, 2024) to codemirror/state 6.5.2 (Feb 3, 2025).

Between those versions were some changes to improve height calculations with regards to the HeightOracle and viewports. Something there introduced an issue where when splitting panes containing the Codemirror 6 editor, the new pane would briefly have a HeightMap height of Infinity which would result in downstream calculations of the offset and value parameters turning into NaN.

This would happen when enabling linewrapping, since linewrapping allows each line height to be variable. The end result was that only partial lines were visible in the editor, or the editor was unscrollable and stuck at the bottom of the document.

Unfortunately I don't have a minimal reproduction of this, but these were the code changes I made in a local patch to codemirror to prevent the display breakage in our application.

I am interested in helping make changes to the original repository. Please tell me what I can do to assist.

I was previously on codemirror/view 6.17.1 (Aug 31, 2023) and upgraded to codemirror/view 6.36.3 (Feb 17, 2025). Likewise upgraded from codemirror/state 6.4.1 (Feb 19, 2024) to codemirror/state 6.5.2 (Feb 3, 2025). Between those versions were some changes to improve height calculations with regards to the HeightOracle and viewports. Something there introduced an issue where when splitting panes containing the Codemirror 6 editor, the new pane would briefly have a HeightMap height of Infinity which would result in downstream calculations of the offset and value parameters turning into NaN. This would happen when enabling linewrapping, since linewrapping allows each line height to be variable. The end result was that only partial lines were visible in the editor, or the editor was unscrollable and stuck at the bottom of the document. Unfortunately I don't have a minimal reproduction of this, but these were the code changes I made in a local patch to codemirror to prevent the display breakage in our application. I am interested in helping make changes to the original repository. Please tell me what I can do to assist.
marijnh commented 2025-07-04 09:23:21 +02:00 (Migrated from github.com)

I would really prefer to have a way to reproduce this, so that we can find the root of the issue. I'm not a fan of this kind of bandaid-style solution, where a bad value computed elsewhere is suppressed, rather than fixed.

Are you able to test whether attached patch maybe address this?

I would really prefer to have a way to reproduce this, so that we can find the root of the issue. I'm not a fan of this kind of bandaid-style solution, where a bad value computed elsewhere is suppressed, rather than fixed. Are you able to test whether attached patch maybe address this?
sfc-gh-klin commented 2025-07-07 20:58:09 +02:00 (Migrated from github.com)

Thank you for the quick response. I agree that having a reproduction would be better. It is unlikely I can provide one this week. I will try out your patch (that 2-line change to HeightOracle and ViewState) and report back.

Thank you for the quick response. I agree that having a reproduction would be better. It is unlikely I can provide one this week. I will try out your patch (that 2-line change to HeightOracle and ViewState) and report back.
sfc-gh-klin commented 2025-07-14 21:58:00 +02:00 (Migrated from github.com)

Yes, the attached patch does seem to address the problem. I tested it out by getting rid of my changes, then pnpm patching those two lines of yours on top of 6.36.3. When I set breakpoints on those two changes and execute the window split in our app, it hits this change:

                refresh = lineHeight > 0 && oracle.refresh(whiteSpace, lineHeight, charWidth, textHeight, Math.max(5, contentWidth / charWidth), lineHeights);

The other change does not get hit in the breakpoint.

That's really great to see that the patch most likely works. I haven't been able to reproduce this in a smaller setup, apologies.

Yes, the attached patch does seem to address the problem. I tested it out by getting rid of my changes, then pnpm patching those two lines of yours on top of 6.36.3. When I set breakpoints on those two changes and execute the window split in our app, it hits this change: ``` refresh = lineHeight > 0 && oracle.refresh(whiteSpace, lineHeight, charWidth, textHeight, Math.max(5, contentWidth / charWidth), lineHeights); ``` The other change does not get hit in the breakpoint. That's really great to see that the patch most likely works. I haven't been able to reproduce this in a smaller setup, apologies.
marijnh commented 2025-07-15 08:36:04 +02:00 (Migrated from github.com)

Great. I've published these changes in 6.38.1

Great. I've published these changes in 6.38.1

Pull request closed

Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
codemirror/view!72
No description provided.