1153237 - Ctrl+Shift+Right arrow doesn't select last word in contenteditable element

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Piotr]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150402191859

Steps to reproduce:

Try to create empty html file. Insert a div with contenteditable attribute set to "true".
 After created div please insert some html node, it can be span 
Write some random text in contenteditable div. Try to select subsequent words with ctrl+alt+right arrow key shortcut and deselect them in ctrl+alt+left arrow.
You have this situation recreated in simple js fiddle:
https://jsfiddle.net/yszqhoz3/1/


Actual results:

last word cannot be selected with ctrl+alt+right arrow


Expected results:

last word should be properly selected just as previous words

Comment 1

[Andrey]

Also ran into this defect, but using ctrl+shift+right arrow, not ctrl+alt+right arrow, could be that "alt" is just a typo.

Comment 2

[Jaromir Dudek]

Same issue here and as was mentioned <ctrl> + <right arrow> (of course + <shift> for selecting text - same behavior), not <alt>.

Seems that this is hapening only in some specific html/js text elements.
For example in this post (when I'm creating it) is everything OK, but for example gmail - compose mail there is visible this bug - last word not possible to select.

FF version 39.0.3 on Windows 7 Enterprise.

Comment 3

[Alin Dornik]

I have experience the same, useing <ctrl> + <shift> + <right arrow> the last word is not selected on a div with contenteditable set on true unless just after the closing div I have a textarea and is not hidden.
In this case selection is not working.
<span><div contenteditable="true">text to select</div><textarea style="display: none;"></textarea></span>
In this case selection is working fine. Also if I add height:0px or opacity:0 on textarea.
<span><div contenteditable="true">text to select</div><textarea></textarea></span>

I'm having this problem in FF version 41.0.2 on Windows 7. Same FF version on linux is not generating this problem.

Comment 4

[Justin [:JW_SoftvisionQA]]

Hello all, is this still a problem for you all? 

I tried reproducing on the newest nightly build 45.0a1 build id 20151112030238 on windows 8.1 and 10.

Comment 5

[Alin Dornik]

Hey Justin,

I've tried the latest nightly build 45.0a1 on Win7 and Win 8.1 and the problem is still there.

Comment 6

[Justin [:JW_SoftvisionQA]]

Ok. I will pass this up to a developer.

Comment 7

[arni2033]

More generally:   Ctrl+Right doesn't move cursor to the very end of last word in [contenteditable]
This issue is presented for a LONG time on vk.com (russian social network; 75+ million users)

Comment 8

[arni2033]

Regression range: 
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=be4ba3d5ca9a&tochange=74edfbf0e6a3
This is not very precise, and I heard that some people are able to continue bisection on local builds, so I'm leaving keyword "regressionwindow-wanted"

Comment 9

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ac55dd11daec&tochange=05b7e79b688e

Regressed by: Bug 1077515

Comment 10

[Jonathan Kew [:jfkthame]]

The problem here shows up only on Windows because of two factors: (a) it depends on layout.word_select.eat_space_to_next_word being set to true, which is (by default) only the case on Windows; and (b) it occurs only if the keystrokes are mapped to the new physically-oriented cmd_{Move,Select}* commands, which are used on Windows, whereas for Cocoa and Gtk the NativeKeyBindings code in widget/ maps keystrokes to the logical cmd_selectWordNext etc.

What happens is that when we try to select the last word, the nsFrameSelection::MoveCaret call fails because it is trying to move to the beginning of the next word (i.e. "eating" the intervening space), but there is no "next word" to be found.

On platforms where we're generating cmd_selectWordNext, this is handled by PresShell::WordMove(), which sees the failure and falls back to CompleteMove() to advance to the end of the text, but PhysicalMove doesn't handle this and so the select (or move) command simply fails.

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: Testcase for failure to select last word in a contenteditable div when using physical select-by-word command

Here's a testcase that should exhibit the failure across all platforms.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: nsFrameSelection::PhysicalMove should fall back to CompleteMove if an intra-line move fails to execute completely

And with this, the testcase now passes.

Comment 13

[Jonathan Kew [:jfkthame]]

Try run with the testcase & patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=672a89c178ad

Testcase alone (expected to fail): https://treeherder.mozilla.org/#/jobs?repo=try&revision=a091633aceb1

Comment 16

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f12e55b099e582b4bf2aba52be817d07f883b61
Bug 1153237 - nsFrameSelection::PhysicalMove should fall back to CompleteMove if an intra-line move fails to execute completely. r=roc

https://hg.mozilla.org/integration/mozilla-inbound/rev/c206ee0a4809e515294aee998fa0b249a7fe1a16
Bug 1153237 - Testcase for failure to select last word in a contenteditable div when using physical select-by-word command. r=roc

Comment 17

[Jonathan Kew [:jfkthame]]

Comment on attachment 8703727 [details] [diff] [review]
nsFrameSelection::PhysicalMove should fall back to CompleteMove if an intra-line move fails to execute completely

Approval Request Comment
[Feature/regressing bug #]: bug 1077515

[User impact if declined]: For Windows users, ctrl-rightarrow fails to move cursor or extend selection across last word of an editable element in some cases (see also description in duplicate bugs).

[Describe test coverage new/current, TreeHerder]: New mochitest landed along with the patch.

[Risks and why]: Minimal risk, patch just adds fallback handling for a failure case that we're currently leaving unhandled.

[String/UUID change made/needed]: n/a

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Piotr, could you please verify this issue is fixed as expected on Nightly build from 1/5/2016 or 1/6/2016? Thanks!

Comment 19

[Jonathan Kew [:jfkthame]]

It won't be fixed on Nightly 1/5, as the patch hasn't yet merged to m-c AFAICS. I hope it'll get merged shortly, in time for 1/6.

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7f12e55b099e
https://hg.mozilla.org/mozilla-central/rev/c206ee0a4809

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8703727 [details] [diff] [review]
nsFrameSelection::PhysicalMove should fall back to CompleteMove if an intra-line move fails to execute completely

Taking it in Aurora45. Denying for Beta44 as this issue has been around for past two releases and this point forward in Beta44 cycle, I am only taking fixes that improve stability or are critical regressions.

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7366d77f97e9
https://hg.mozilla.org/releases/mozilla-aurora/rev/9ef49ec1f925

Comment 24

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

I'm porting the test here to mochitest-plain for bug 1269209 so that it runs on e10s too.  Would it be fine to replace the use of commandDispatcher here with synthesizeKey?  According to comment 10, this will still test the original bug on Windows.  Or, do you know of another way of testing it that works in a plain mochitest?

Comment 25

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Never mind, we need to get arbitrary commands working in plain mochitests for other tests anyway.