GetmeUK / ContentTools

A JS library for building WYSIWYG editors for HTML content.
http://getcontenttools.com
MIT License
3.95k stars 393 forks source link

Keyboard navigation is driving me nuts #487

Open cubiclesoft opened 6 years ago

cubiclesoft commented 6 years ago

And I can't figure out the cause for most of these issues (Firefox 59.0, Windows 10 on a desktop, wired keyboard/mouse). I'm finally to the point where I can actually use ContentTools in production and I keep encountering multiple issues that are extremely difficult to replicate in order of severity:

  1. While typing in the middle of a normal paragraph tag, the keyboard caret will jump mid-word to the middle of a completely different paragraph. That makes my preferred 70wpm typing speed tenuous at best. I'm having to slow way down to about 20wpm to avoid writing sentences in the middle of other sentences.

  2. Arrow key navigation sometimes gets "stuck". I'll want to move to the next paragraph but pressing the down/right arrow keys does nothing even though I am visually at the end of the paragraph. If I press enter (which creates a new paragraph) and then backspace (which deletes the newly created paragraph), it'll fix the problem and I can resume navigating without issue.

  3. Arrow key navigation skips blocky things like images and videos and gets "stuck" when the focus is on the blocky things. Pressing enter will create a new paragraph and get things back to normal, but that's not really a great fix.

  4. Ctrl + Left/Right Arrow keys inexplicably jumps all over the place. I'll use Ctrl + left/right to navigate the caret one word at a time and suddenly I'll end up somewhere in the previous paragraph. Might be related to the first issue.

  5. Ctrl + Home/End and Page Up/Down don't work as I expect. I do kind of like that the Ctrl + Home/End key combo will jump to the paragraph start/end if I'm in the middle of the paragraph but it should move to the first/last element respectively when the user is already at the start/end of the paragraph. Page up and down should attempt to page up and down through the elements while maintaining keyboard caret focus.

I suspect most of the issues are related to the Undo/Redo stack. I also know that I'm 15 commits behind the latest but, as I look at the commit log on master, I'd be taking on the HTML copy and paste can of worms that's just been opened - to the best of my knowledge, no one has ever figured out how to handle pasted Word HTML (and I wrote TagFilter). How stable is the HTML copy and paste logic - are you confident that it is ready-ready or are you still working on it?

cubiclesoft commented 6 years ago

Hooray! (Hooray?) After a lot of fiddling around, I can (finally) reliably replicate the second issue:

The bug can be replicated on the demo, which I assume is running some recent-ish flavor:

http://getcontenttools.com/demo

I'm still struggling with trying to replicate the first issue. I'll keep plugging away at it. When the first issue crops up, it really makes a mess of things.

Edit: After some digging around the source code, I've figured out what causes the second issue. To debug I added:

console.log(selection)

To Text.prototype._keyRight. And:

console.log(this.content.length());
console.log(this.content.html());

To Text.prototype._syncContent. Here is the output to the Javascript console:

0  <-- new paragraph

1  <-- pressed 'a'
a
1  <-- pressed 'enter'
a
0

1  <-- pressed 'b'
b
1  <-- pressed left arrow
b
1  <-- pressed left arrow
b
1
a
3  <-- pressed space
a <br>
4  <-- pressed 'c'
a c<br>
Object { _from: 3, _to: 3 }  <-- pressed right arrow
4
a c<br>

Pressing right arrow never gets past the <br> element at the end of the string (i.e. can't reach a value of 4) and so the caret never moves to the next paragraph because it isn't actually at the end of the string.

cubiclesoft commented 6 years ago

Possibly related to the first issue but can be easily replicated:

(a) When the second down arrow is hit, the caret is at the start of the first line of the second paragraph. The third down arrow moves to the second line at the position that the caret was at on the second line of the first paragraph. The editor skips moving the caret to the middle of the first line. I assume part of the issue here has to do with how switching the paragraph being moved into over to contenteditable functions.

(b) The horizontal caret position is forgotten when changing lines/paragraphs particularly with going back up to the previous paragraph. It also takes four keypresses to return to the same line as before.

This bug/these bugs can be replicated on the demo.

Most code/text editors seem to only update their memorized internal horizontal caret position when a key other than up or down is pressed.

There's also an issue here where changing paragraphs with just up and down arrow keys on rare occasions results in placing the caret at the beginning of the line when moving in-paragraph but the last line remains unaffected. I seem to be able to replicate it more frequently when there are lots of red underlines from incorrectly spelled words, which suggests a timing issue of sorts.

As to replicating the first issue, I've only had it happen twice so far in my attempts to replicate it. During the most recent "success," I pressed enter in the middle of the paragraph that I was in and the editor put the new paragraph before the current paragraph and did not split the paragraph in which I had pressed enter. The caret was moved to the new paragraph too. I assume if I had pressed a different key at that moment, the caret would have jumped to the previous element on the page and started inserting text there as described in my earlier comment. Given the difficulty I'm having reproducing this, I suspect a timing issue is involved. There are approximately 20 setTimeout() and setInterval() calls scattered throughout the source code so it could be one of those OR it could be the web browser. I don't know at this point - I haven't even looked at the code yet.

anthonyjb commented 6 years ago

@cubiclesoft thanks for all the research, my guess is that the problems with carat position will lie wither within the ContentSelect library, or as you've flagged with the Undo/Redo stack is triggering a timing issue.

Unless enacted however the History stack shouldn't set (restore) the position of the cursor. History should be passive - however if it's querying for carat position it's possible that ContentSelect is doing something that loses or modifies the carat position on query.

It's great to have a solid way to reproduce the issue, just to confirm using the latest CT version on Chrome I can't reproduce the issues you've provided instructions for above (accept losing the position of the cursor when moving up/down into a new paragraph which isn't coded for). I can, however, reproduce these issues on Firefox which leads me to believe that the issues are most likely related to ContentSelect not handling carat position correctly in FireFox.

There's no specific handling of Home/End and Page Up/Down currently, but there's no reason what you've suggested couldn't be added - though it's probably better raised as a separate enhancement issue.

anthonyjb commented 6 years ago

@cubiclesoft as for HTML support it's a nightmare of course, but it's working well enough that we routinely use it now to move content between sites/pages for clients, however, it's not perfect and we limit what's supported, in particular there are extra limitations when copying from word.

However, I don't believe any of the more recent changes (most of which of specific to HTML C&P) would change the behaviour here - as I say I think the issues are ContentSelect related.

cubiclesoft commented 6 years ago

Yes, I use Firefox as my primary web browser. I hadn't gotten around to testing other browsers.

cubiclesoft commented 6 years ago

I just submitted a couple of fixes for this issue. Keyboard navigation is a bit better now. I still can't replicate the first problem with any consistency. The pull requests fix the second issue (the spurious br tags) and the third issue (navigation to/from text, images, and video elements).

I briefly attempted to fix horizontal navigation tracking across elements but quickly realized that's a mini-project in itself and gave up. There are just too many nuances to deal with at the moment.