Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double caret blink in Sonoma #59

Closed
wants to merge 3 commits into from

Conversation

cyanzhong
Copy link

@cyanzhong cyanzhong commented Nov 8, 2023

This issue only reproduces in macOS Sonoma (reproducible with https://codemirror.net/try/ too). Please check out the video.

double.blink.mov

It seems by setting caretColor to cm-line doesn't hide the native caret, I fixed it in my project by adding:

.cm-content {
  caret-color: transparent !important;
}

// or through the theming system

Feel free to close and make your fix as I don't know what's the best practice. I am just sharing an example. Thanks.

@cyanzhong
Copy link
Author

@marijnh fyi.

@marijnh
Copy link
Member

marijnh commented Nov 9, 2023

Which Safari version is that? I don't have Sonoma. Safari 17.1 doesn't seem to have this issue.

@cyanzhong
Copy link
Author

cyanzhong commented Nov 9, 2023

I am also using 17.1:

image

You need to look into it closely, the native caret is bolder than the one we have in CodeMirror:

Screen.Recording.2023-11-09.at.8.42.01.AM.mov

@marijnh
Copy link
Member

marijnh commented Nov 10, 2023

Even with a colored, almost transparent CodeMirror cursor, I am not seeing the native cursor when drawSelection is enabled.

@cyanzhong
Copy link
Author

cyanzhong commented Nov 10, 2023

I am not sure, probably this is a Sonoma issue instead of just WebKit. In macOS Sonoma, Apple introduced quite a few changes to text input, such as:

Inline predictions

image

Input source indicator

Screen.Recording.2023-11-10.at.1.15.05.AM.mov

Caret breathing effect

Looking into my video, you'll see fade in/out effects of the native caret.

I believe some of them are not tied to WebKit, more likely a system-wide change.

Again, I am not sure, just sharing what I observed. What I can confirm is that the above features are added to all input controls, including WebKit, TextKit, and so on.

Thanks.

@marijnh
Copy link
Member

marijnh commented Nov 10, 2023

Okay. Really odd that CSS rule matching, which is very much a browser-internal thing, would work differently for the same browser version on different OS versions. But I guess your fix is mostly harmless. Just to make absolutely sure we're not working from confused assumptions here, could you look at this page and confirm that you are getting a visible cursor when you focus the top editable element, but not in the bottom one?

@cyanzhong
Copy link
Author

cyanzhong commented Nov 10, 2023

I am seeing the opposite (which I think is expected?):

image image

Also, I tried this (added a background-color):

<style>
  .cm-content, .cm-line, .cm-line2 { caret-color: transparent; background-color: red; }
</style>

<div class=cm-content contenteditable=true><div class=cm-line>This should have a visible cursor (on Sonoma)</div></div>

<div class=cm-content2 contenteditable=true><div class=cm-line2>This should not</div></div>

I am getting:

image image

The background-color matches both, but the caret-color only matches the first, I cannot explain.

Maybe it's time to report a WebKit bug.

@cyanzhong
Copy link
Author

cyanzhong commented Nov 10, 2023

I think a proper fix would be just moving the caret hiding logic from cm-line to cm-content (the root element of contenteditable):

image

It works for Sonoma and other clients, and seems not a weird code, hiding both seems safer though.

@cyanzhong
Copy link
Author

cyanzhong commented Nov 10, 2023

We also have a default theme like this:

image

I am assuming this is for those don't use drawSelection and the change is not a problem (probably we should have hidden caret in cm-content in the first place?).

@marijnh marijnh closed this in df237c8 Nov 10, 2023
@marijnh
Copy link
Member

marijnh commented Nov 10, 2023

I am seeing the opposite (which I think is expected?):

Um, yeah, I flipped the styles around, you're right.

Attached patch changes this in a slightly more minimal way. We'll also want to keep a style for .cm-line in case a theme overrides it. And yes, caret-color in themes only takes effect when you don't use drawSelection—otherwise .cm-cursor is what is showing the cursor.

@marijnh
Copy link
Member

marijnh commented Nov 10, 2023

Followed up with cf5e5a9 because my initial patch was completely broken.

@cyanzhong
Copy link
Author

Thank you!

@cyanzhong cyanzhong deleted the patch-2 branch November 10, 2023 17:19
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.

2 participants