Cursor position jumps after resize and when using line wrapping #1787

jasonsanjose · 2013-08-27T17:11:14Z

Possible duplicate of #1642

Demo: http://jasonsanjose.github.io/uploads/word-wrap-bug.html.

Highlights

Using height: 100% on body
Not calling refresh and relying on the built-in window resize handling

<style type="text/css">
  body {
    margin: 0;
    padding: 0;
    height: 100%;
  }
  #content {
    width: 100%;
    height: 100%;
  }
</style>

...

<script>
  var editor = CodeMirror(document.getElementById("content"), {
    value: document.getElementById("code").value,
    lineNumbers: true,
    lineWrapping: true
  });

  editor.setSize("100%", "100%");
</script>

Steps to reproduce:

Resize the browser window so that there is no vertical scrolling and the width of the window is just barely big enough to avoid a horizontal scrollbar (line 6 is the longest line)
Place the cursor on any text in line 11 (although any line should work)
Watch the cursor position carefully, then vertically resize the window smaller
Observe the cursor position moves as the vertical scrollbar shows/hides and while line 6 wraps

Reproduced on Chrome 29 and Canary 31.

The text was updated successfully, but these errors were encountered:

marijnh · 2013-08-29T09:26:44Z

Are you seeing it stuck in the wrong place, or only briefly flickering at a wrong position and then being fixed again? (I only see the latter when I try to reproduce this.)

jasonsanjose · 2013-08-29T15:24:48Z

@marijnh I actually see both behaviors.

marijnh · 2013-08-29T15:53:36Z

Despite what you wrote in adobe/brackets#4564, I am still not able to reproduce this.

jasonsanjose · 2013-08-29T16:00:36Z

Oh, my mistake. I thought that's what you meant in your prior comment.

marijnh · 2013-08-30T08:20:52Z

Okay, let me elaborate. I see the jumping during resize. This is intentional. If the correct position is recomputed for every resize event, size dragging becomes terribly slow. So it delays a bit and only does the work every 200 ms. I actually believe this change was in response to a problem in Brackets, but I can't find the issue/thread for it right now.

RaymondLim · 2013-09-04T17:56:36Z

@marijnh
I wonder whether you're testing on Mac. If you are, then you need to turn on "Show scroll bars" to "Always" in "General" tab of system preferences. Otherwise, you won't be able to reproduce it. Also the cursor will stay on the next line only when you decrease the window height (not on increasing the height) small enough to trigger the vertical scroll bar showing up.

marijnh · 2013-09-05T11:55:58Z

Updated Chrome on my MacBook to 29, and really tried to follow your instructions to the letter, but I am still not seeing this happen.

RaymondLim · 2013-09-05T16:29:15Z

@marijnh Thanks for trying. I'll try to describe it on my own words and see you can reproduce it on your end.

Open Jason's page (http://jasonsanjose.github.io/uploads/word-wrap-bug.html) in a browser. (I think any browser will do. I tried it in Safari and Chrome on Mac, and Firefox and Chrome on Win 7.)
Make sure that the width of the browser window is just a few pixels more than the longest line on line 6 and no scroll bars showing in the browser. Note that you need to turn on Show scroll bars as Always on Mac.
Set the cursor inside any word on a line below line 6.
Try to resize the browser window by holding down left mouse button on the bottom edge of the browser window and move mouse up slowly as you watch the hair cursor.
As soon as you see the cursor jumps to the next line, release your mouse button. The cursor should stay on the same position as the one before you let go of the mouse button.

0b10011 · 2013-09-05T17:31:34Z

I can reproduce this on Ubuntu 13.04, Firefox 23. I only have the outline resize when resizing windows (so size/page doesn't update until I let go of the edge). To reproduce, I just dragged the bottom edge of the window up a few pixels at a time until the cursor appears on the wrong line. When I do this, Line 6 wraps for a split second, the following lines move down, and the cursor remains in the same spot (previous line). Then, Line 6 unwraps, the following lines move back up, and the cursor updates to where it was when Line 6 was wrapped. Calling editor.refresh() or typing updates the cursor position to where it should be. The character appears in the original position, so it's only the visual cursor div that's in the wrong place--the internal "cursor" remains in the correct position.

So, seems to me this is a race condition with the visual placement of the cursor div and the visual updating of the editor. Something like:

Window resized
Editor resizes (and wraps line 6) assuming scrollbar
Cursor's visual position is calculated and saved
Editor resizes (and unwraps line 6) due to no scrollbar
Cursor div is moved to position from step 3

There should be a step between 4 and 5 that recalculates the scrollbar position before it's displayed, or a step 6 in which the cursor div is moved to the new position again (with the second saved position).

Original position

Shows for split second (after resizing)

Final position

marijnh · 2013-09-06T09:10:17Z

I've finally managed to reproduce this using Firefox (still no luck with Chrome). Investigating.

Issue #1787

marijnh · 2013-09-06T09:57:21Z

Could you test whether the attached patch helps?

RaymondLim · 2013-09-06T14:22:24Z

Yes, the patch indeed fixes the issue. Thanks.

marijnh · 2013-09-06T14:34:05Z

Wondeful.

jasonsanjose · 2013-09-06T17:05:07Z

@RaymondLim I actually was able to reproduced it still in my example http://jasonsanjose.github.io/uploads/word-wrap-bug.html with the same frequency as before.

0b10011 · 2013-09-06T17:35:33Z

I actually was able to reproduced it still in my example

As was I.

RaymondLim · 2013-09-06T17:48:39Z

Yeah, you both are correct. I still can reproduce it with Jason's page in Chrome and Safari browser on Mac. The one case that it fixes is only in Brackets when resizing from the bottom edge.

Issue #1787

marijnh · 2013-09-09T08:34:18Z

Try again with attached patch. I had isolated the problem in a test case that used setSize, but the case where the window resize handler triggered the bug was slightly different, and my fix didn't work for that case. It should now.

0b10011 · 2013-09-09T13:35:44Z

Works for me!

RaymondLim · 2013-09-09T20:13:28Z

Thanks @marijnh. The new patch fixes the issue both in browsers and in Brackets. But it can also cause Brackets to hang. The hang seems to be caused by the infinite loop due to this condition check (cm.options.lineWrapping && oldWidth != cm.display.scroller.clientWidth). So if I change lineWrapping to false in Brackets, then we can avoid this hang. I don't have a reproducible case with CodeMirror demo pages though. For now my only reproducible case is running Brackets unit tests with changes in 7ea6f43.

Update: Below is the call stack that I got from running unit test in Brackets.

getRect (codemirror.js:5465)
measureLineInner (codemirror.js:1106)
measureLine (codemirror.js:1019)
cursorCoords (codemirror.js:1201)
updateSelectionCursor (codemirror.js:790)
updateSelection (codemirror.js:769)
updateDisplay (codemirror.js:417)
endOperation (codemirror.js:1376)
window.CodeMirror.withOp (codemirror.js:1417)
CodeMirror (codemirror.js:70)
Editor (Editor.js:375)
createMockEditorForDocument (SpecRunnerUtils.js:358)
createMockEditor (SpecRunnerUtils.js:382)
createTestEditor (Editor-test.js:64)
(anonymous function) (Editor-test.js:87)
jasmine.Block.execute (jasmine.js:1024)
jasmine.Queue.next_ (jasmine.js:1842)
jasmine.Queue.start (jasmine.js:1795)
jasmine.Spec.execute (jasmine.js:2122)
jasmine.Queue.next_ (jasmine.js:1842)
jasmine.Queue.start (jasmine.js:1795)
jasmine.Suite.execute (jasmine.js:2267)
jasmine.Queue.next_ (jasmine.js:1842)
jasmine.Queue.start (jasmine.js:1795)
jasmine.Suite.execute (jasmine.js:2267)
jasmine.Queue.next_ (jasmine.js:1842)
onComplete (jasmine.js:1838)
jasmine.Suite.finish (jasmine.js:2224)
(anonymous function) (jasmine.js:2268)
jasmine.Queue.next_ (jasmine.js:1852)
onComplete (jasmine.js:1838)
jasmine.Suite.finish (jasmine.js:2224)
(anonymous function) (jasmine.js:2268)
jasmine.Queue.next_ (jasmine.js:1852)
onComplete (jasmine.js:1838)
jasmine.Spec.finish (jasmine.js:2096)
(anonymous function) (jasmine.js:2123)
jasmine.Queue.next_ (jasmine.js:1852)
(anonymous function) (jasmine.js:1832)

marijnh · 2013-09-10T16:23:55Z

The problem is almost certainly the continue added by my patches causing an infinite loop because the width of the editor doesn't stabilize. I have a semi-formal proof that this can not happen, but since it's happening for you, I guess there's something wrong with my reasoning. Could you apply the below patch, run your tests, and tell me what you see on the console? (It should no longer hang, since I limited the loop to 10 iterations. Might still be slow.).

diff --git a/lib/codemirror.js b/lib/codemirror.js
index a14ce53..503d259 100644
--- a/lib/codemirror.js
+++ b/lib/codemirror.js
@@ -409,7 +409,8 @@ window.CodeMirror = (function() {
   function updateDisplay(cm, changes, viewPort, forced) {
     var oldFrom = cm.display.showingFrom, oldTo = cm.display.showingTo, updated;
     var visible = visibleLines(cm.display, cm.doc, viewPort);
-    for (;;) {
+    console.log("starting updateDisplay loop");
+    for (var cut = 0; cut < 10; ++cut) {
       var oldWidth = cm.display.scroller.clientWidth;
       if (!updateDisplayInner(cm, changes, visible, forced)) break;
       updated = true;
@@ -417,6 +418,7 @@ window.CodeMirror = (function() {
       updateSelection(cm);
       updateScrollbars(cm);
       if (cm.options.lineWrapping && oldWidth != cm.display.scroller.clientWidth) {
+        console.log("width changed from", oldWidth, "to", cm.display.scroller.clientWidth, " -- doing a hard re-measure");
         forced = true;
         continue;
       }

RaymondLim · 2013-09-10T16:59:03Z

Yes, no longer hang but slow as you said. Below is two separate sections of console output.

starting updateDisplay loop codemirror.js:413
width changed from 26 to 76 -- doing a hard re-measure codemirror.js:422
width changed from 76 to 102 -- doing a hard re-measure codemirror.js:422
width changed from 102 to 128 -- doing a hard re-measure codemirror.js:422
width changed from 128 to 154 -- doing a hard re-measure codemirror.js:422
width changed from 154 to 180 -- doing a hard re-measure codemirror.js:422
width changed from 180 to 206 -- doing a hard re-measure codemirror.js:422
width changed from 206 to 232 -- doing a hard re-measure codemirror.js:422
width changed from 232 to 258 -- doing a hard re-measure codemirror.js:422
width changed from 258 to 284 -- doing a hard re-measure codemirror.js:422
width changed from 284 to 310 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
width changed from 310 to 336 -- doing a hard re-measure codemirror.js:422
width changed from 336 to 362 -- doing a hard re-measure codemirror.js:422
width changed from 362 to 388 -- doing a hard re-measure codemirror.js:422
width changed from 388 to 414 -- doing a hard re-measure codemirror.js:422
width changed from 414 to 440 -- doing a hard re-measure codemirror.js:422
width changed from 440 to 466 -- doing a hard re-measure codemirror.js:422
width changed from 466 to 492 -- doing a hard re-measure codemirror.js:422
width changed from 492 to 518 -- doing a hard re-measure codemirror.js:422
width changed from 518 to 544 -- doing a hard re-measure codemirror.js:422
width changed from 544 to 570 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
width changed from 570 to 722 -- doing a hard re-measure codemirror.js:422
width changed from 722 to 874 -- doing a hard re-measure codemirror.js:422
width changed from 874 to 1026 -- doing a hard re-measure codemirror.js:422
width changed from 1026 to 1178 -- doing a hard re-measure codemirror.js:422
width changed from 1178 to 1330 -- doing a hard re-measure codemirror.js:422
width changed from 1330 to 1482 -- doing a hard re-measure codemirror.js:422
width changed from 1482 to 1634 -- doing a hard re-measure codemirror.js:422
width changed from 1634 to 1786 -- doing a hard re-measure codemirror.js:422
width changed from 1786 to 1938 -- doing a hard re-measure codemirror.js:422
width changed from 1938 to 2090 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
width changed from 26 to 52 -- doing a hard re-measure codemirror.js:422
width changed from 52 to 78 -- doing a hard re-measure codemirror.js:422

and can continue up to a very large number

width changed from 13626 to 13953 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
width changed from 13953 to 14280 -- doing a hard re-measure codemirror.js:422
width changed from 14280 to 14607 -- doing a hard re-measure codemirror.js:422
width changed from 14607 to 14934 -- doing a hard re-measure codemirror.js:422
width changed from 14934 to 15261 -- doing a hard re-measure codemirror.js:422
width changed from 15261 to 15588 -- doing a hard re-measure codemirror.js:422
width changed from 15588 to 15915 -- doing a hard re-measure codemirror.js:422
width changed from 15915 to 16242 -- doing a hard re-measure codemirror.js:422
width changed from 16242 to 16569 -- doing a hard re-measure codemirror.js:422
width changed from 16569 to 16896 -- doing a hard re-measure codemirror.js:422
width changed from 16896 to 17223 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
width changed from 17223 to 17578 -- doing a hard re-measure codemirror.js:422
width changed from 17578 to 17933 -- doing a hard re-measure codemirror.js:422
width changed from 17933 to 18288 -- doing a hard re-measure codemirror.js:422
width changed from 18288 to 18643 -- doing a hard re-measure codemirror.js:422
width changed from 18643 to 18998 -- doing a hard re-measure codemirror.js:422
width changed from 18998 to 19353 -- doing a hard re-measure codemirror.js:422
width changed from 19353 to 19708 -- doing a hard re-measure codemirror.js:422
width changed from 19708 to 20063 -- doing a hard re-measure codemirror.js:422
width changed from 20063 to 20418 -- doing a hard re-measure codemirror.js:422
width changed from 20418 to 20773 -- doing a hard re-measure codemirror.js:422

marijnh · 2013-09-10T17:09:50Z

Well, there's certainly something unexpected going on there -- there's no way redrawing or remeasuring the code should cause the width to increase on every iteration. And why does it start so small? For me, when wrapping is turned on, I see only two widths -- the one where the vertical scrollbar is there, and the one where it's not. Can you inspect the actual DOM and see if the thing it contains during these tests is an actual sane editor? Can you see what is determining the width of the scroller element and how it could be constantly changing?

RaymondLim · 2013-09-10T18:23:41Z

It looks like we always start out with an empty editor window and show the gutter (line numbers) by default. If I set lineNumbers to false, then I don't see the width of scroller element constantly changing.

Below is what I see in the console with lineNumbers default to false.

starting updateDisplay loop codemirror.js:413
width changed from 26 to 50 -- doing a hard re-measure codemirror.js:422
width changed from 50 to 26 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
width changed from 26 to 152 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
width changed from 26 to 152 -- doing a hard re-measure codemirror.js:422
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
starting updateDisplay loop codemirror.js:413
width changed from 26 to 152 -- doing a hard re-measure codemirror.js:422

marijnh · 2013-09-11T11:07:22Z

Still, no matter what the gutter is doing, with the default css, the scroll element's width should be fixed, so its clientWidth would only differ depening on its vertical scrollbar. I think you have some css rule that's causing trouble.

njx · 2013-09-21T00:08:26Z

It looks like the problem was that our unit tests create a mock container for the editor, and that mock container is absolutely positioned with no explicit width, so it's 0-width by default. This didn't cause a problem before, but I can imagine that it would cause issues after this change, since the editor itself will be the one that's determining the width of its container, so I can imagine how it could get into some kind of feedback loop (though I didn't dig into it to figure out exactly where the loop was causing form). I was able to work around this just by setting an explicit width on the mock container in our unit tests, since that's more realistic anyway.

I don't know whether this is something that actually needs to be fixed in CM; this use case certainly isn't likely in normal real-world usage, but perhaps it's possible for someone to have a 0-width or small-width container in some edge case (e.g. at the start of an animation?), and it would be bad for CM to hang in that case. (I didn't actually create a test case with vanilla CodeMirror to see if that would trigger the same hang, though.)

njx · 2013-09-21T00:10:50Z

BTW, I didn't verify the original issue was fixed since I haven't been involved in the details--I'll put up a pull request in Brackets and have @RaymondLim verify that we're all set and can close this CM issue.

RaymondLim · 2013-09-21T02:34:53Z

Verify that the original issue (with Quick Docs or Quick Edit in Brackets) is also fixed with the latest update to CodeMirror.

Issue #1787

marijnh · 2013-09-23T10:15:48Z

I've added a check to prevent problems in (more or less nonsensical) cases like this. Closing.

njx · 2013-09-23T20:20:39Z

Seems fine, thanks.

Issue codemirror#1787

This was referenced Aug 27, 2013

Cursor shows up on next line after collapsing Quick Doc (with word wrap enabled) adobe/brackets#4564

Closed

incorrect cursor position on wrapped lines for some users with Chrome #1642

Closed

lkcampbell mentioned this issue Aug 29, 2013

Japanese text + single-byte spaces + line wrap = display and selection go wonky adobe/brackets#4992

Closed

lkcampbell mentioned this issue Aug 29, 2013

Word wrapped editor + hyphenated text = wrong cursor position #1791

Closed

marijnh added a commit that referenced this issue Sep 6, 2013

Prevent corrupted measurements due to vanishing/appearing scrollbar

c8c7fc1

Issue #1787

marijnh closed this as completed Sep 6, 2013

marijnh added a commit that referenced this issue Sep 9, 2013

Amend c8c7fc1 to handle window resize-tiggered updated

7ea6f43

Issue #1787

marijnh reopened this Sep 10, 2013

jasonsanjose mentioned this issue Sep 12, 2013

Unit tests hang after CodeMirror patches for issue #1787 adobe/brackets#5179

Closed

njx mentioned this issue Sep 21, 2013

Update CodeMirror for sprint 32. Avoid hang by setting width on mock editor holder in unit tests. adobe/brackets#5283

Merged

marijnh added a commit that referenced this issue Sep 23, 2013

Ensure scrollbar/width instability doesn't cause infinite loop

d2b2ed7

Issue #1787

marijnh closed this as completed Sep 23, 2013

anaran pushed a commit to anaran/CodeMirror that referenced this issue Feb 22, 2014

Prevent corrupted measurements due to vanishing/appearing scrollbar

9161e52

Issue codemirror#1787

anaran pushed a commit to anaran/CodeMirror that referenced this issue Feb 22, 2014

Amend c8c7fc1 to handle window resize-tiggered updated

1e56f91

Issue codemirror#1787

anaran pushed a commit to anaran/CodeMirror that referenced this issue Feb 22, 2014

Ensure scrollbar/width instability doesn't cause infinite loop

b57bba6

Issue codemirror#1787

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cursor position jumps after resize and when using line wrapping #1787

Cursor position jumps after resize and when using line wrapping #1787

jasonsanjose commented Aug 27, 2013

marijnh commented Aug 29, 2013

jasonsanjose commented Aug 29, 2013

marijnh commented Aug 29, 2013

jasonsanjose commented Aug 29, 2013

marijnh commented Aug 30, 2013

RaymondLim commented Sep 4, 2013

marijnh commented Sep 5, 2013

RaymondLim commented Sep 5, 2013

0b10011 commented Sep 5, 2013

marijnh commented Sep 6, 2013

marijnh commented Sep 6, 2013

RaymondLim commented Sep 6, 2013

marijnh commented Sep 6, 2013

jasonsanjose commented Sep 6, 2013

0b10011 commented Sep 6, 2013

RaymondLim commented Sep 6, 2013

marijnh commented Sep 9, 2013

0b10011 commented Sep 9, 2013

RaymondLim commented Sep 9, 2013

marijnh commented Sep 10, 2013

RaymondLim commented Sep 10, 2013

marijnh commented Sep 10, 2013

RaymondLim commented Sep 10, 2013

marijnh commented Sep 11, 2013

njx commented Sep 21, 2013

njx commented Sep 21, 2013

RaymondLim commented Sep 21, 2013

marijnh commented Sep 23, 2013

njx commented Sep 23, 2013

Cursor position jumps after resize and when using line wrapping #1787

Cursor position jumps after resize and when using line wrapping #1787

Comments

jasonsanjose commented Aug 27, 2013

marijnh commented Aug 29, 2013

jasonsanjose commented Aug 29, 2013

marijnh commented Aug 29, 2013

jasonsanjose commented Aug 29, 2013

marijnh commented Aug 30, 2013

RaymondLim commented Sep 4, 2013

marijnh commented Sep 5, 2013

RaymondLim commented Sep 5, 2013

0b10011 commented Sep 5, 2013

Original position

Shows for split second (after resizing)

Final position

marijnh commented Sep 6, 2013

marijnh commented Sep 6, 2013

RaymondLim commented Sep 6, 2013

marijnh commented Sep 6, 2013

jasonsanjose commented Sep 6, 2013

0b10011 commented Sep 6, 2013

RaymondLim commented Sep 6, 2013

marijnh commented Sep 9, 2013

0b10011 commented Sep 9, 2013

RaymondLim commented Sep 9, 2013

marijnh commented Sep 10, 2013

RaymondLim commented Sep 10, 2013

marijnh commented Sep 10, 2013

RaymondLim commented Sep 10, 2013

marijnh commented Sep 11, 2013

njx commented Sep 21, 2013

njx commented Sep 21, 2013

RaymondLim commented Sep 21, 2013

marijnh commented Sep 23, 2013

njx commented Sep 23, 2013