adobe / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
33.25k stars 7.63k forks source link

Repaint & scrolling problems with Page Up / Page Down #1030

Closed peterflynn closed 12 years ago

peterflynn commented 12 years ago

Bug 1

  1. Place cursor on first line of a long file (I used InlineEditorProviders-test.js)
  2. Page Down
  3. Look for cursor
  4. Hit Left/Right arrow

Result: (3) no blinking cursor visible, (4) view scrolls up one line to reveal cursor

Bug 2

  1. Go back to the top, and place the cursor on a line about halfway down the viewport
  2. Page Down
  3. Look for cursor
  4. Hit Left/Right arrow

Result: (3) no blinking cursor visible, (4) blinking cursor appears on a line in middle of viewport (view does not scroll at all)

Bug 3

  1. Go back to the top, and place the cursor on a line about halfway down the viewport
  2. Shift+Page Down
  3. Look for selection highlight
  4. Hit Shift+Up/Down

Result: (3) no selection visible, (4) selection appears covering top ~1/2 of the viewport

Bug 4

  1. Go back to the top, and place the cursor on a line about halfway down the viewport
  2. Hold Shift+Down arrow until you start scrolling

Result: after it starts scrolling, the top of the selection range moves upward one or more lines

This one only seems to repro inconsistently -- it appears to be timing dependent.

Bug 5

  1. Open a file that's > 1 but < 2 screenfulls tall
  2. Go to top and place cursor on a line near the bottom of the viewport
  3. Shift+Page Down
  4. Scroll back up via mouse

Result: (3) selection highlight misaligned with text -- ends in the middle of a line's vertical bounds, (4) selection highlight extends way higher than you started from (often to the first line in the file)

peterflynn commented 12 years ago

@njx I'm pretty sure this was working before the CodeMirror merge

njx commented 12 years ago

Is this similar to #1025?

peterflynn commented 12 years ago

I added a few more cases where the selection highlight is even more screwed up

peterflynn commented 12 years ago

Oops, yes -- I think #1025 is "Bug 2" here. So this is a superset. Should we close the other one?

njx commented 12 years ago

This should be fixed by https://github.com/adobe/CodeMirror2/pull/61. Note that in case 3, you might end up with the cursor on a different line after the page up--this is due to #1036--but that's not a new issue. (The highlighting should be consistent with the cursor position though, so if that's not the case let me know.)

njx commented 12 years ago

FBNC to @peterflynn -- please update to the latest adobe/CodeMirror2 master.

peterflynn commented 12 years ago

I can no longer repro bugs 1-3 and bug 5, but bug 4 still occurs for me. Here are more detailed steps:

Bug 4

  1. Open InlineEditorProviders-test.js and size the window to be about 73 1/2 lines tall
  2. With viewport at top of file, place the cursor in the middle of on line 62 (e.g. at the end of the token "allFiles")
  3. Hold Shift+Down arrow until you start scrolling

Result: after it starts scrolling, the top of the selection range moves upward one or more lines. It seems to jiggle around, moving up a bit, pulling back, moving up again. Depending on when you let go of the down arrow, the selection might be back in a good state or it might still be wrong.

It's purely a display problem: if I hit Ctrl+C or delete, it's clear that the selection range starts where I'd put the cursor even if the UI now shows it starting several lines earlier.

It almost seems as if the selection is being redrawn at a different pace from the scroll position: the selection moves upward more often, while the scroll position update is chunkier... leading to them being out of sync whenever the scroll position update lags the selection position update (but occassionaly being in sync, right after the scroll pos has updated).

It seems to be timing dependent, anyway: just tapping Shift+Down repeatedly does not repro the issue.

peterflynn commented 12 years ago

Also, I've been unable to repro this in the sprint-9 build.

njx commented 12 years ago

Note: to repro Peter's bug, you need the window sized fairly large (e.g. full screen on a big monitor). This creates a race condition between the keyboard handling and the scroll event handling, so multiple keyboard events are processed before the scroll event is handled.

njx commented 12 years ago

FBNC to @peterflynn (again :)) -- this should generally fix any remaining scrolling issues around keyboard navigation and selection.

peterflynn commented 12 years ago

Excellent! Confirmed: all the above bugs are now fixed.