Prosemirror: Spell Checker Problems

[WesleyKapow]

I believe this behavior has always existed but I tested it with 0.8.1 AND with the current basic demo. I've also tested the behavior on a regular contenteditable div (not ProseMirror) and it seems to work correctly.

In Chrome:

1. Underlines show up while typing a word (expected: only shows up after moving away from word)
2. Typing on the same line with mis-spelled words causes their underlines to flash.

In Safari:

1. Underlines don't always show up. Seems like hitting enter after a word or clicking in the middle of a word and then else where will cause them to show up.
2. Underlines disappear when typing on same line.

[marijnh]

This is due to the fact that ProseMirror makes and renders the changes to the document, instead of letting user events through, and it is probably unavoidable for an editor built with this architecture (until browser decide to give scripts control over spell-checking -- but don't hold your breath for that one /cc @johanneswilm).

[johanneswilm]

@marijnh What we will get is a beforeinput event that gives a target range specifying what the user asked to be changed and what the replacement word would be.

What there is opposition to among some browser vendors is to give scripts direct access to the spell checker functionality. This is because at least Safari uses the system dictionary which can have custom entries which could give clues about the user. So it ends up being a privacy issue.

We have talked a bit about what other options would be available. One could be that one can mark certain regions as being in need of spell checking or mark a range as having been checked already. If you guys have ideas about that, feel free to share them.

[WesleyKapow]

So the issue is that ProseMirror is disrupting the normal flow of user events within contenteditable? Is it possible to not have it do this for normal character insertion?

If there's no good way to get the default spell checker working then ProseMirror should probably come with spell check disabled by default and with a comment about default spell checking not working.

Anyone know of any good JS spell checkers? I was most likely going to need to go down this path anyway in order to allow for a custom contextmenu (like google docs).

[marijnh]

I think the behavior you describe is not _broken_ per se -- it does allow users to use the spell checker, it's just a bit more aggressive about underlining than usual.

[WesleyKapow]

Maybe in chrome but not in safari as the underlines don't always appear.

[johanneswilm]

Anyone know of any good JS spell checkers?

The main problem with this is download size of the dictionaries. This problem will (obviously) solve itself with time, but for the time being downloading 1-10MB is still not something most will want their website visitors to go through.

The alternative is using webservices -- but those are either paid for, or you run it yourself and will mean you need to calculate that in when figuring out what kind of server you need.

[johanneswilm]

Looks like the US English dictionary here is about 700kb: http://www.chrisfinke.com/files/typo-demo/ . That may be acceptable under some circumstances, especially if it's cached.

I think another thing that browser makers may be convinced of, if they accept that this a big enough issue for users, could be to allow for access to systems dictionaries without modifications -- in other words, the dictionaries as they are shipped with the OS without anything they user may have added or edited.

[WesleyKapow]

Sounds like due to the way ProseMirror works this is sort of a WONTFIX/CANTFIX and there's now an option to disable default spell check.

Closing.

P.S: The mark range system is perfect for do it yourself spell checking as you can mark a range with a className that adds the underlines!

[rafbm]

I am very interested in the development of ProseMirror. I have been comparing it closely with other modern libraries and truly think it has the best foundation to become the most powerful editor out there. However right now it falls short from a UX perspective, preventing spellcheck in Safari and acting unexpectedly on iOS, see #463 and #234 (double-tap space issue is closed but hasn’t actually been fixed in PM itself).

Here is a visual example of the current Safari behavior with a spellcheck-enabled ProseMirror:

The problem is pretty obvious. @marijnh’s position on the matter (emphasis mine):

This is due to the fact that ProseMirror makes and renders the changes to the document, instead of letting user events through, and _it is probably unavoidable for an editor built with this architecture_

I think it’s worth showing how seamless the Safari spellcheck experience is with Trix:

Though I have yet to dig in the internals of ProseMirror and Trix to see exactly how each updates their view, the DOM tree highlights that appear in the Chrome console seem to indicate that ProseMirror replaces full <span> elements whereas Trix updates text nodes directly.

Considering this, plus the fact that Trix _does_ use an architecture where all key events are intercepted, applied on a document model and re-rendered into the DOM (see text in GIF above), there must be a way for ProseMirror to update text nodes directly and be on par with Trix regarding spellcheck and iOS behavior without any hack.

I would love to have Marijnh’s thoughts on this. Is there really some core principle to ProseMirror that would prevent adopting this approach?

Note that Quill and Draft.js also provide the exact same Safari and iOS behaviors as Trix.

[marijnh]

Given your analysis, it appears that browser react much better to text nodes being updated than to whole spans being replaced, when it comes to spell-checking behavior. That is hopeful. We could experiment with using something like incremental-dom or our own simpler implementation of a similar algorithm to do the actual rewriting of DOM nodes, and see if that makes things better. I'm reopening this issue to further track that work.

[marijnh]

Though I have yet to dig in the internals of ProseMirror and Trix to see exactly how each updates their view, the DOM tree highlights that appear in the Chrome console seem to indicate that ProseMirror replaces full elements whereas Trix updates text nodes directly.

@rafbm In the next release, ProseMirror will also use an update model where it directly updates text nodes (by setting their nodeValue), but unfortunately, this _does not help_ at all in Safari, where you still don't get spell-checking underlines until you move the cursor. If you want to do more research into what Trix & co are actually doing, that might be useful.

[marijnh]

For those interested, here's the most minimal update-dom-directly-on-keypress editor implementation I could come up with. In Safari, this suppresses all spell checking, so I suspect that the editors in which spell check works are letting keypress events go through after all.

<!doctype html>
<meta charset=utf8>

<div style="border: 1px solid silver; padding: 5px; white-space: pre-wrap" contenteditable=true><p>hello</p></div>

<script>
  function setSel(node, offset) {
    let s = getSelection(), r = document.createRange()
    r.setEnd(node, offset)
    r.collapse(false)
    s.removeAllRanges()
    s.addRange(r)
  }

  document.querySelector("div").addEventListener("keypress", e => {
    e.preventDefault()
    let s = getSelection(), node = s.focusNode
    if (node.nodeType == 3) {
      let offset = s.focusOffset
      node.nodeValue = node.nodeValue.slice(0, offset) + String.fromCharCode(e.charCode) +
        node.nodeValue.slice(offset)
      setSel(node, offset + 1)
    }
  })
</script>

[rafbm]

You are totally right Marijn. Sorry for providing an incorrect analysis in the first place.

Considering this, plus the fact that Trix does use an architecture where all key events are intercepted, applied on a document model and re-rendered into the DOM […]

It appears this is actually _not true for simple character inputs_. Other editors (see Trix and Quill code) are using a MutationObserver (with polyfill via Mutation Events for IE9-10). In Trix’s case, it’s simple to see they catch childList and characterData-type mutations, such as this example where I typed “g” at the end of a node:

Trix then compares the MutationRecord’s oldValue with a simple diffing algorithm, which they use to update their internal document model, without any further change to the DOM beside the original, un-prevented keypress/input event.

That seems to be the only way to get spell check as they do in Safari, however I was also wrong stating that Trix provides a “_seamless_ spell check behavior in Safari”. With just a little more testing, it’s clear that whenever they do need to prevent default events and apply them programatically, Safari’s pickiness kicks in and you lose spell check underlines you had:

For anything beyond simple character input (here inserting a <strong> and appending a <br>), Trix replaces the whole node content and the spell check experience is hurt.

So I guess having truly seamless spell check in Safari may be a pipe dream. However I do believe their MutationObserver strategy where keypress/input events are not prevented on simple character input is what makes Trix smoother on iOS.

[marijnh]

With just a little more testing, it’s clear that whenever they do need to prevent default events and apply them programatically, Safari’s pickiness kicks in and you lose spell check underlines you had

Yeah, I was about to say something similar. ProseMirror could, at the cost of some efficiency (parsing, diffing, and fixing up the DOM is a bunch of work) let plain keypress events through, but there are _many_ situations where it will have to touch the DOM, which seems to cause Safari's spell checker to immediately run and hide. So the experience could be a little better, but it'll be relatively poor still. If you want, I can provide this behavior (ignoring keypress) as an option.

[rafbm]

I think letting keypress / input events go through would be a big benefit for all users. Having this be the default behavior rather than an option would make sense to me.

While it’s definitely OK to consider Safari a lost cause, doing some further testing on a spellcheck-enabled ProseMirror in Chrome shows that the experience is almost worst than without spell check. Since text nodes are fully rewritten, underlines appear while typing partial words (before hitting Space) and always keep flashing on every further keystroke:

The above is latest ProseMirror on http://prosemirror.net/ with the spellcheck="false" attribute manually removed.

This, as well as most iOS issues if I’m not mistaken, sound like a good reason to explore a strategy similar to Trix / Quill for simple character inputs.

[marijnh]

Letting keys go through won't, by itself, do anything about the flashing, and we're _already_ letting keypress through on iOS.

[marijnh]

I've pushed a bunch of patches (https://github.com/ProseMirror/prosemirror-view/compare/e47c3b6e4ec5...448dee780a8945cf3880cd7dbf25a9f84b7875de) that take a hint from Trix and uses MutationObserver (which I had started with but given up on, but which is viable again with ProseMirror's current DOM management technique) to get a more precise and reliable view on which DOM nodes changed. This has made letting the browser handle typing and backspacing (when not across block boundaries) viable and relatively efficient again. I've come up with a way to have simple editing, when the browser's behavior agree with what ProseMirror would have done, happen with zero programmatic redraws, so that should make Safari spell check kinda work. It still resets as soon as you add a mark or do something similar, but it's better than it was before.

I think this is about as far as we can go with this. We do, in the end, need control over marks, and our model of marks is completely different from the browser's execCommand approach. It might be possible to cover some cases with hacky uses of execCommand, but that'd still be only partial, and introduce a lot of complexity that's probably not worth it. So I'm closing this, but feel free to continue discussing or to open new issues for related problems.

[johanneswilm]

It might be possible to cover some cases with hacky uses of execCommand, but that'd still be only partial, and introduce a lot of complexity that's probably not worth it.

Yes, please do not start using execCommand. ExecCommand will likely be gone in future versions of contenteditable, so this is not an area I would recommend adding dependencies in.

[rafbm]

Thanks very much for efforts in this direction Marijn! It does seem to be as far as we can go without adding unwanted complexity. It’s also very nice to see pretty much all if (browser.ios) conditions removed. Fewer code branches, fewer bugs!

I’m stumbling on a few core bugs while fiddling with the demo. I pulled all latest versions of the repos and removed all content from the initial node:

<div id=content style="display: none"></div>

On the first line, Backspace does nothing at all. On the lines after, it deletes the whole line, rather than a single character. And the cursor gets moved to the beginning of the previous line.

[rafbm]

And here’s an exception that gets thrown (probably an edge case). It’s when I backspace the apostrophe after the browser replaced it from ' to ’:

[WesleyKapow]

Working on upgrading from 0.12 to 0.14 and am running into the same things @rafbm ran into.

[rafbm]

Other buggish behavior here. I’m doing Alt+Fn+Backspace (ie. Alt+Delete) to forward-delete words. The first 3 times (inside sentence) it works, but then doing so before the hard break of the first line, it duplicates the content of the next line.

I’m reporting all these bugs because they look different, but let me know if you’re already implementing a magic fix for all of them. I’ll stop with the GIFs. 😃

[marijnh]

Working on upgrading from 0.12 to 0.14 and am running into the same things @rafbm ran into.

None of this has been released so probably you're seeing a different problem.

[marijnh]

I've pushed some patches that seem to fix the issues you found, @rafbm . Please open new issues for further problems, so that we don't make a mess of this issue.

[mitar]

Just one additional idea here, which I think is closer to what Google Docs does: do spell-checking on the server. So at least in collaborative editing scenario, I am already sending every change to the server. Server could do its regular state update, then extract text, do spell checking, if anything is bad, use marks to mark that as failing spell checking, add one more version, and send both original version and additional version with spell checking back to all users. This would have an added benefit that all users would see regions of the document with failed spell checking, and even have per-document or per-project exceptions (custom dictionary).