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

Selection is not stable in IE11 (probably due to bookmarks) #33

Closed caffodian closed 8 years ago

caffodian commented 8 years ago

Creating a bookmark in IE11 modifies the selection ranges.

In the render method, before going in and marking spans, we create a bookmark at the current selection. This causes the really simple "ignore the current selection" code in the Wordwalker to fail.

There are a few ways out of this:

1) Really optimistically: because we no longer care about marking the current word, we no longer need to set bookmarks in order for the cursor to not move (very unlikely this actually works, but worth trying) 2) get the selection before creating the bookmark, since we only care about it for the purposes of marking the text node that we don't care about. this may still not work since the bookmark might do something bad.
3) take another pass at the "find the currently selected word" code. It should also ignore empty trailing children instead of just always looking at the last child. (I am most sure that this will work, though it feels the most hacky)

caffodian commented 8 years ago

Option 2 won't always work if the selection is not the first word, since marking words will invalidate the selection (if it's not bookmarked)

Option 1 definitely doesn't work, either.

Something that might make more sense with some thought:

caffodian commented 8 years ago

i can't think of any other way to make this sane in IE. pretty annoying that adding a bookmark actually modify the selection. argh.

caffodian commented 8 years ago

This might also be an uber special case. when awake: need to verify if this happens mid-block as well.

caffodian commented 8 years ago

Current bug: when your selection isn't in the range, we don't have a way of determining if a dom walker ended due to guard, or exhaustion.

caffodian commented 8 years ago

We should generalize this to support multiple bookmarks. Mostly because the process of doing so will probably remove a lot of repetition and make the code actually human readable.

We really should be setting it up the same way each time. The difference is:

caffodian commented 8 years ago

The "we immediately hit a bookmark" is actually a weird special case that breaks our usual assumptions (normally we have a text node that we can get the parent of)

caffodian commented 8 years ago

If we just store the last bookmark hit (rather than just whether we hit a bookmark) , we can simplify the "start a new walker after this bookmark" logic

caffodian commented 8 years ago

Quick spot check with modern IE: there are some tests that fail due to break insertion but the walker tests are passing, which is a good sign.