1258308 - <textarea> displays caret on wrong line after selecting \n on the second line

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[arni2033]

STR:
1. Open attached "testcase 1"
2. Click in the second line, press End to move cursor to the end of the line
 [you will get "asdf\nghjk[caret]\nzxcv" in textarea]
3. Press Shift+Right to select "\n"
4. Press Right key to move cursor to the end of text selection
5. Type "x"

Result:       
 After Step 4 cursor is displayed at the end of the 2nd line
 After Step 5 the typed letter ("x") appears in the beginning of the 3rd line

Expectations:
 After Step 4 cursor should be displayed at the start of the 3rd line

Comment 2

[arni2033]

>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160320030409

Comment 3

[Mats Palmgren (inactive)]

Ah right, the fix in bug 1237236 is using GetPrimaryFrame so it only really
works on the first line.  We need to lookup the actual continuation that
renders aParentNode/aOffset...

Comment 4

[Jet Villegas (inactive)]

(In reply to Mats Palmgren (:mats) from comment #3)
> Ah right, the fix in bug 1237236 is using GetPrimaryFrame so it only really
> works on the first line.  We need to lookup the actual continuation that
> renders aParentNode/aOffset...

Can you rename this bug with the correct diagnosis? We've now got multiple bugs with similar symptoms in the name. Thanks!

Comment 5

[Mats Palmgren (inactive)]

This is just a variation that my fix in bug 1237236 didn't catch.

Comment 6

[Mats Palmgren (inactive)]

Attachment: fix

I guess we could skip the first do_QueryFrame now, but nsCaret::GetFrameAndOffset
looks relatively expensive so it's probably worth avoid calling it unless we
know we have a text frame.

Comment 7

[Mats Palmgren (inactive)]

Attachment: tests

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0ddd899ef57e

Comment 8

[(no longer active)]

Comment on attachment 8733409 [details] [diff] [review]
tests

Review of attachment 8733409 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/tests/test_reftests_with_caret.html
@@ +170,5 @@
>    tests.push([ 'bug966992-1.html' , 'bug966992-1-ref.html' ]);
>    tests.push([ 'bug966992-2.html' , 'bug966992-2-ref.html' ]);
>    tests.push([ 'bug966992-3.html' , 'bug966992-3-ref.html' ]);
>    tests.push(function() {SpecialPowers.pushPrefEnv({'clear': [['layout.css.overflow-clip-box.enabled']]}, nextTest);});
> +  tests.push([ 'bug1258308-1.html' , 'bug1258308-1-ref.html' ]); // it seems VK_END doesn't work on Android

Hmm.  bug664087-*.html seem to use this just fine.  But this is fine if you want to land it as is.

Comment 9

[Mats Palmgren (inactive)]

Odd... the failures looked identical on OSX and Android -- after I changed VK_END to
Accel+VK_RIGHT for OSX the test worked fine so I assumed it was the same problem on
Android, only I couldn't figure out what key combo to use there...

Anyway, I'll land it without Android testing for now.
(and I added a "maybe?" to the test comment quoted above)

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82c36cd54626
https://hg.mozilla.org/integration/mozilla-inbound/rev/99a181de0c57

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/82c36cd54626
https://hg.mozilla.org/mozilla-central/rev/99a181de0c57

Comment 12

[arni2033]

STR in comment 0 is Fixed on:   Win7_64, Nightly 48, 32bit, ID 20160324030447

But I still can reproduce the same issue if I select the last \n if it's the last symbol in textarea.
I.e. if I press Shift+Right then Right in textarea "A\nB[caret]\n"

I already filed that case as bug 1237286 (Or should I file a separate bug not involving Shift+Enter?)

Comment 13

[Mats Palmgren (inactive)]

It may be the same underlying bug as bug 1237286, but please file a separate bug just in case,
with a testcase and STR.  Mark it as blocking 1237286 to make a link between the two.

Thanks for verifying these fixes on Windows.