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

Incorrect (visual) cursor in Safari if atom node is last node in line #1165

Closed
2 of 5 tasks
philippkuehn opened this issue May 6, 2021 · 9 comments
Closed
2 of 5 tasks

Comments

@philippkuehn
Copy link

philippkuehn commented May 6, 2021

Issue details

When using atom nodes it’s not possible to place the cursor after it when the node is the last node in a line. You can see that in the offical dinos example. When placing a dino at the end of a line, Safari renders the cursor at the start of a line. If I type something, the content will be placed after the dino (which is correct) but it feels weird because of the wrong (visual) cursor rendered by Safari.

Steps to reproduce

ProseMirror version

prosemirror-view: 1.18.4

Affected platforms

  • Chrome
  • Firefox
  • Internet Explorer
  • Desktop Safari
  • iOS Safari

Screenshots / Screencast (Optional)

RPReplay_Final1620282287.MP4
@robbertbrak
Copy link

As mentioned in the referenced issue, this is not just an issue on iOS. It also happens in Safari 14.1 on desktop.

@philippkuehn
Copy link
Author

Ah you are right! So maybe this will be easier to debug.

@philippkuehn philippkuehn changed the title Incorrect (visual) iOS cursor if atom node is last node in line Incorrect (visual) cursor in Safari if atom node is last node in line May 6, 2021
@robbertbrak
Copy link

I noticed that the issue goes away if you set the contenteditable attribute of the atoms (mentions / dinos) to true instead of false.

Of course, setting contenteditable="true" leads to other issues, such as that it then becomes possible (in some cases) to place the cursor inside the atom and edit the text content of the mention. But perhaps something can be done in ProseMirror to prevent this from happening (e.g. by detecting when the cursor moves inside an atom).

marijnh added a commit to ProseMirror/prosemirror-view that referenced this issue Jun 22, 2021
…ne on Safari

To avoid a bug where the browser draws the cursor at the start of the line.

FIX: Work around a Safari bug where it draws the cursor at the start of the line
when it is after an uneditable node at the end of the line.

Issue ProseMirror/prosemirror#1165
@marijnh
Copy link
Member

marijnh commented Jun 22, 2021

This is definitely a browser bug, but attached patch tries to work around it by injecting an additional src-less <img> element, which seems to stop the bug from triggering. I'd appreciate it if those affected could test this thoroughly to see if it produces any regressions for them, since this kind of kludges is always a bit scary. I'll hold off on releasing it until I get some feedback.

@mmorearty
Copy link

Thanks @marijnh. Looks like there's a separate bug in this. It does fix the bug described in this issue, but if the cursor moves to an empty paragraph (including if the entire document is empty), there is a crash: TypeError: undefined is not an object (evaluating 'lastChild.dom')

In addTextblockHacks on this line:

    if (result.safari && lastChild.dom.contentEditable == "false")

Here's a way to repro with the Dino example:

  • git clone https://github.com/ProseMirror/website.git
  • Edit the prosemirror-view line in package.json to say "prosemirror-view": "ProseMirror/prosemirror-view" (this is an npm trick to tell it to install directly from GitHub instead of from the npm registry)
  • npm install
  • then run and test

marijnh added a commit to ProseMirror/prosemirror-view that referenced this issue Jun 22, 2021
@marijnh
Copy link
Member

marijnh commented Jun 22, 2021

Argh, my bad, yes, that was an obvious bug. Attached patch should address it.

@mmorearty
Copy link

Looks good. In my app I tested things such as cursor movement, selection, clicking, typing, and pasting near this, and everything was fine.

@philippkuehn
Copy link
Author

Thanks for the fix. Looks good to me!

@srsudar
Copy link

srsudar commented Jun 16, 2023

I'm seeing this after another type of atom node, also in Safari. The node has contentEditable = "false", like I'd expect to be caught by this fix. Does anyone have any debugging tips?

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

No branches or pull requests

5 participants