Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clicking on multi-line selection inside inline editor jumps scroll position way downward #13059

Open
core-ai-bot opened this issue Aug 31, 2021 · 17 comments
Open

Clicking on multi-line selection inside inline editor jumps scroll position way downward #13059

core-ai-bot opened this issue Aug 31, 2021 · 17 comments

Comments

@core-ai-bot
Copy link
Member

Issue by peterflynn
Friday Feb 13, 2015 at 07:42 GMT
Originally opened as adobe/brackets#10590

New repro steps with Getting Started project

  1. Open Getting Started project and open index.html
  2. Scroll down to the <h3> on line 61 ("Quick Edit for CSS and JavaScript")
  3. Invoke Quick Edit on the <h3> tag
  4. Within the inline editor, create a text selection that spans > 1 line
  5. Click the selection

If it doesn't repro, scroll so the <h3> is further down from the top of the viewport before step 3, or make your window less tall.

Old repro steps with JS inline editor

  1. Open Editor.js in the Brackets source
  2. In the Editor constructor, place the cursor on the _getModeFromDocument() call and press Ctrl-E
  3. Inside the inline editor, drag to select some text
  4. Click on the selected text

Result: scroll position jumps down about 250 lines, and focus is lost from the editor

Expected: selection cleared and cursor remains in inline editor

This looks like a regression due to PR #9584 -- if I disable text drag & drop via preferences, the bug goes away. CC@MarcelGerber

I'm only able to repro this in the JS inline editor -- not in the CSS inline editor. If it occurred in the CSS inline editor I think we'd want to block the 1.2 release until a fix is available (or make drag & drop off by default). If it really only repros in the JS inline editor, I'm less sure... Seems like we should investigate to at least see if there's an easy fix, though.
@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 08:08 GMT

The "scroll" event callstack shows nothing -- it's being triggered natively, not via JS. The scroll happens immediately on mousedown (no mousemove, mouseup, or click needed).

Looking at a Timelines capture, codemirror responds to the mousedown with leftButtonStartDrag(), which sets display.scroller.draggable = true;. Then there's an almost instananeous blur-focus event pair, and by the time that's over it looks like the scroll is a foregone conclusion. There are three scroll events logged: first on CM's hidden textarea, then on some div (can't tell which one), then on the .CodeMirror-scroll div.

My guess is that CM's hidden textarea is getting positioned incorrectly, and when it's focused the browser then tries to scroll it into view (which is standard behavior when focusing any element that's out of view).

The focus change itself may be a bug, since focus appears to be nowhere useful after you mouseup. Or it may be expected, while the badly positioned focus element is the bug. Either way, seems like it's likely to be a bug on CM's side, not ours...

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 08:33 GMT

The blur event is changing focus from the inline editor's hidden textarea to the inline editor's .CodeMirror-scroll (i.e. the thing draggable=true was just set on). The focus event immediately after it is focus moving right back the other way. Since the scroller div and hidden textarea are both already in view, it's unclear why these focus changes would trigger any sort of scroll.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 09:05 GMT

Turns out the draggable=true is a red herring. The momentary focus change is enough to cause the problem even with the draggable stuff commented out in CM.

With drag & drop disabled, CodeMirror calls preventDefault() on mousedown, blocking any momentary focus change. With drag & drop enabled, CodeMirror can't do that, because it will also block the browser from starting a drag event (this is one of my pet peeves with the DOM Events API -- preventDefault() is an overly generic catch-all that can block several things when you only want to block one of them). Hence the focus is briefly lost on every mousedown. (This is also why a barely-noticeable flicker is seen, a small but unavoidable manifestation of bug #141).

But the momentary focus change happens on every mousedown in any CM widget when drag & drop is enabled. Yet it only causes spurious scrolling in a JS inline editor -- hopefully poking at that will lead to a fix.

@core-ai-bot
Copy link
Member Author

Comment by MarcelGerber
Friday Feb 13, 2015 at 18:59 GMT

Just FYI, I'm completely unable to repro this bug.
I've followed the steps multiple times and every time I get the expected behaviour: the dragged selection becomes a cursor.

I'm on Windows 8.1, using the latest (downloaded) prerelase shell and with brackets@27a6b79e994ccecc9dd6126a28ddffbb499c755b

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 20:03 GMT

@MarcelGerber It repros 100% of the time for me on release branch (4131f1b) with no extensions installed and no local code changes. Happens with both the official 1.1 shell build and my locally built master/1.2 shell build.

I'm on Mac 10.9 -- it's possible this could be a Mac-only bug... Can anyone else confirm?

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Friday Feb 13, 2015 at 20:25 GMT

@peterflynn and@MarcelGerber I can reproduce it on mac, but only if you select text that spans over multiple lines in step 3.

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Friday Feb 13, 2015 at 20:32 GMT

I don't see it on Windows 8.1

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Friday Feb 13, 2015 at 20:38 GMT

I can't reproduce it on windows either. Selecting the highlighted with double-click always ends up with selecting only the word that clicked on.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 21:19 GMT

Good observation@RaymondLim! Same for me -- only happens if selection spans > 1 line.

As a test, I wired up the inline editor to always just show the top of index.html, and it still happens (so the content of the inline editor need not be a JS file). I also rewired the provider to respond 100% of the time, and it still happens even when the outer editor is just a plain text file.

This means it does repro even with the CSS inline editor, in the right circumstances -- making this higher priority than I'd originally thought.

The bug is sensitive to a number of factors: how far below the top of the viewport the inline editor is opened (too high up and it won't repro; lower down the scroll jumps gets larger), window height (taller window requires opening the editor lower down before it will start to repro), and how many lines are spanned by the selection inside the inline editor (selecting more lines makes it start reproing higher up, and it scrolls down more lines total). That last bit reminds me of #3376, which also varies with how many lines you have selected -- and which we never got to the bottom of either :-(

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 21:20 GMT

@RaymondLim What's the connection to double-clicking when you select? I'm able to repro by simply dragging a selection (or even using Shift-arrows) -- it doesn't seem to matter how you create the selection initially...

@core-ai-bot
Copy link
Member Author

Comment by RaymondLim
Friday Feb 13, 2015 at 21:28 GMT

Disregard double-clicking. I was referring to the step 4 to click inside the selected text. Single click just sets the cursor to where you click on Windows, but double-click always selects the word you click on.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 23:17 GMT

I'm able to reproduce this running Brackets in-browser, but so far I'm not able to reproduce this with a simpler standalone CM testcase (https://gist.github.com/peterflynn/0f6cd0af22edb543f56f).

That's both good and bad news: it suggests there's not a fundamental problem with CM's drag/drop mechanism, but the bad news is it could be very hard to tease apart what's different between Brackets and the simpler CM testcase...

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Feb 13, 2015 at 23:59 GMT

Spoke too soon! If you scroll things just right, it will in fact repro in the standalone CM testcase too -- score!

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Saturday Feb 14, 2015 at 00:22 GMT

Filed codemirror/codemirror5#3081

@core-ai-bot
Copy link
Member Author

Comment by le717
Thursday Mar 05, 2015 at 20:44 GMT

@peterflynn@MarcelGerber's upstream fix at codemirror/codemirror5#3100 has been merged, so add this to v1.3 milestone, I guess?

@core-ai-bot
Copy link
Member Author

Comment by nethip
Friday Apr 17, 2015 at 04:56 GMT

I am moving this out to 1.4 as we are going to pull latest version of code mirror only in 1.4

@core-ai-bot
Copy link
Member Author

Comment by nethip
Wednesday Jul 01, 2015 at 05:00 GMT

@rroshan1 Could you check if this is fixed with the latest version of code mirror in Brackets?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@core-ai-bot