ether / etherpad-lite

Etherpad: A modern really-real-time collaborative document editor.
http://docs.etherpad.org/
Apache License 2.0
16.47k stars 2.84k forks source link

Line-break causing weird cursor behaviour - Waiting on Triage #3087

Open renebrain opened 7 years ago

renebrain commented 7 years ago

Hi guys,

during my testing of etherpad i realized, that the cursor has some strange beviour when the following action occurs than the cursor is becoming weird: You add a long word into etherpad so that the lin breaks in the and of etherpad and creates a new line with the text continuing. When you go to the first character of the new line and try to go back with the left arrow of the keyboard he goes up to the previous line and the last character and immediatly jumps back to the second line and the first character. you need to press the left arrow for a long time so that he over comes this part and goes back further. I attached a gif with the bug from the testinstance.

bug

renebrain commented 7 years ago

So any ideas here? It is still prelevant in the latest version.

lpagliari commented 7 years ago

Hi, @renebrain , could you provide some info about the instance you're using, please?

Thanks

renebrain commented 7 years ago

As you can already see on the top, this is already the test case on https://beta.etherpad.org/. It happens everywhere. On your instance aswell as on our own. For us it happens on Firefox. On Linux, aswell as Windows and Mac. For all the other browsers he just skips the last word in the first row.

lpagliari commented 7 years ago

You didn't mention your test was running on https://beta.etherpad.org/ and your screenshot did not show the URL, so I tried there using Chrome on Mac and could not reproduce. That's why I've asked you the questions above. Thanks for the tests, BTW! We'll take a look on it.

renebrain commented 6 years ago

Any results here?

monkey-pawan commented 6 years ago

Hi, In addition to the solution to this issue, can someone tell me which hook or code is responsible to insert line-break (<br>) on hitting enter/return so I could look around to see if I could find something. On line-break, etherpad forgets the formatting of previous paragraph/line and I want it to continue that way, particularly the line height even if there's no character other than just <br>. thanks in advance.

lpagliari commented 6 years ago

@monkey-pawan the place where a new line is generated is on ace2_inner.js. You can work around that issue by using the aceKeyEvent hook.

monkey-pawan commented 6 years ago

@lpagliari thank you so much for your response. Can you please help me locate where actual insertion of tags in dom is happening, particular tags like <br> and <ol>, I tried and chased it up to performDocumentReplaceRange and setAttributeOnLine respectively then lost. Are actually insertion in dom is happening in Changeset.js? Could you point me to the right place, please? thanks in advance.

lpagliari commented 6 years ago

Hum, I'm not sure about where that is... But if you want to maintain the line format when user presses ENTER, I would suggest you to overwrite the behaviour of ace2_inner on a plugin: when user presses ENTER, your plugin should do the same thing that ace2_inner does, but also it should copy all line attributes from original line to the new line. That would make what you need, I believe.

monkey-pawan commented 6 years ago

I'm trying to add one more list type i.e. "Numbered List" leaving Ordered List as it is. The numbered list would have roman, alpha and decimal on various levels. Apparently, passing doInsertList anything other than "number" would create a <ul> but I want it to generate <ol> so that's what is making me look for some if or switch statement where the code chooses which tag to insert in dom.

P.S. I was not able to reproduce original issue with etherpad version 1.6.1 with given steps but I noticed a few times when a div with magicdomid is created and removed [because it has no characters in it, I guess] repeatedly, which makes the cursor look bouncing back and forth but I can't reproduce the issue every time I want so can't look into it.

JohnMcLear commented 6 years ago

I can replicate in FF on beta.etherpad.org - ignoring most of cruft conversation in this thread tho! :P

JohnMcLear commented 4 years ago

I took another look into this and it's related to:

             scroll.scrollWhenPressArrowKeys(arrowUp, rep, innerHeight);

In ace2_inner.js

comented out = can go down a line (right arrow) uncommented = can go up a line (left arrow)

It's present on no-skin and colibris skins

:D

 this.scrollNodeVerticallyIntoView(rep, innerHeight);

Commenting this changes behavior

Wow then further digging we find commenting out var linePosition = caretPosition.getPosition(); changes the behavior.. a getPosition should not be doing anything that should change state...

Looks like it's something to do with this in caretPosition.js

    if(!rect || rect.height === 0){
      var clonedRange = createSelectionRange(range);

      // as we can't get the element height, we create a text node to get the dimensions
      // on the position
      var shadowCaret = $(document.createTextNode("|"));
      clonedRange.insertNode(shadowCaret[0]);
      clonedRange.selectNode(shadowCaret[0]);

      line = getPositionOfElementOrSelection(clonedRange);
      clonedRange.detach()
      shadowCaret.remove();
    }

Will keep digging..

JohnMcLear commented 4 years ago

Okay so interesting I have a solution that works ONCE then doesn't work again for some lines, it's defo an improvement but obv I haven't formatted anything, just did debug now need to do stuff IRL. Will try polish it asap.

https://github.com/JohnMcLear/etherpad-lite/commit/48fcc6d71f1b6f01a5d32e3d04413ab449629ce6

JohnMcLear commented 4 years ago

Errrr I just discovered that on develop in Chrome the left key is double stepping on line wraps! I'm guessing this is related..

Bumping to serious bug because of the browser impact and because it makes it a really horrible UX.

JohnMcLear commented 4 years ago

Double step is fixed by: https://github.com/ether/etherpad-lite/pull/3831

JohnMcLear commented 4 years ago

I have things mostly fixed but if you go to a line, keystroke right then get to 2nd wrapped line, then keystroke left, you can't keystroke right again. As if it's creating some hidden element and leaving it.

JohnMcLear commented 4 years ago

Right guys, I have pushed my final commit, given time constraints I think this is as stable as I can get it for now and would ask for a call for someone to assist me resolving this further so we don't have the issue of going left > right on wrapped line.

JohnMcLear commented 4 years ago

I'm going to take a stab at test coverage, because we need that

JohnMcLear commented 4 years ago

Writing tests I'm hitting this browser limitation: https://stackoverflow.com/a/19883789/695411

JohnMcLear commented 4 years ago

Tests are in, but with a huge caveat that we can't properly cover this issue, or cover it at all. The sendkeys library doesn't emulate a trigger which is what's causing the bug. So we know a keypress is causing the fault, and it's due to shadow.

I'm going to unassign this from me now because it has as much test coverage and bugfix as I can do.

JohnMcLear commented 4 years ago

Given that this is "mostly" fixed we can push this out of this release and onto another milestone? @muxator

muxator commented 4 years ago

Yep, postponing.

JohnMcLear commented 4 years ago

Just to confirm the initial pull request and fix is in yeah? My fix allowed caret to pass back over line one time then failed.

valerio-bozzolan commented 3 years ago

Any news on this?

webzwo0i commented 3 years ago

Seems to work in chromium, but not in firefox. Will try to reevaluate John's findings from above

valerio-bozzolan commented 3 years ago

(I was trying to test on latest version at https://beta.etherpad.org/ but now it's offline.)