Closed Bug 1258308 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: Selection, defect, minor)

defect
Not set
minor

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: arni2033, Assigned: mats)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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
>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160320030409
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...
Component: Editor → Selection
(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!
Flags: needinfo?(mats)
This is just a variation that my fix in bug 1237236 didn't catch.
Assignee: nobody → mats
Severity: normal → minor
Flags: needinfo?(mats)
Summary: <textarea> displays caret on wrong line after selecting \n, and when I start typing, caret teleports to the next line [3/2] → <textarea> displays caret on wrong line after selecting \n on the second line
Attached patch fixSplinter Review
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.
Attachment #8733407 - Flags: review?(ehsan)
Attachment #8733407 - Flags: review?(ehsan) → review+
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.
Attachment #8733409 - Flags: review+
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)
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/82c36cd54626
https://hg.mozilla.org/mozilla-central/rev/99a181de0c57
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
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?)
Status: RESOLVED → VERIFIED
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.
See Also: → 1259949
You need to log in before you can comment on or make changes to this bug.