PolicyStat / ckeditor-spell-check-plugin-js-dev-edge

User interface enhancements and beta features for http://ckeditor-spellcheck.nanospell.com . This repo is for nanospell developers and trusted partners to modify and fork plugin.js
Other
0 stars 0 forks source link

Spellcheck spans are modifying document HTML #26

Closed winhamwr closed 8 years ago

winhamwr commented 8 years ago

Under some common condition (it's happened in 3 out of 3 edited PolicyStat documents I've checked so far), spell check modifies the HTML, even if the user doesn't use the spellcheck suggestions.

The goal should be to not alter HTML structure. My guess is that the structure is also altered when you accept a suggestion, but I didn't test that.

Some examples of HTML before editor load and then the resulting HTML after the document was saved:

First word spelling suggestion

caffodian commented 8 years ago

I think the surrounding HTML matters, but I'm not sure how yet. The best thing to do is probably to start with the whole p tag as test data.

<p><strong>Josh &amp; Gabe</strong> - started process of eliminating the traditional &quot;Sales Transition Call&quot; then IC team met to refine and establish new Kickoff call guidelines.</p>

Gets mangled.

My attempt at typing "Josh & Gabe" into bold text, then saving and refreshing - didn't mangle the strong tag. Same with various other attempts at good word followed by typo, or ampersand + space followed typo, all inside strong tags.

winhamwr commented 8 years ago

This behavior would interact pretty badly with the autoid stuff (if h# tags are impacted). If a heading got split in two, we'd have ID confusion.

sbaughman commented 8 years ago

I've tracked it down to wrapTypoWithSpan function. When the range contents get extracted for insertion into the created typo span element, the range collapses and somehow the subsequent insertion of the span causes a split in the outer tag.

This only occurs when the text being wrapped is directly against the closing tag. It also does not happen if there is no additional content after the closing tag.

No solution yet, but making progress

sbaughman commented 8 years ago

Update: so it's actually a consequence of how CKEDITOR.dom.range.extractContents works

winhamwr commented 8 years ago

We've decided that we have to be OK with some level of mutation, unless we want to fight very hard against CKEditor internals. We don't want to do that.

25 and #27 (I think) mitigated some of the worst-case examples of this, where we could.