391464 - CTRL-right arrow (or SHIFT-CTRL-right) advance caret (or selection) past first word on next line

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect, P2)

Description

[skierpage]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007080805 Minefield/3.0a8pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007080805 Minefield/3.0a8pre ID:2007080805

If you press CTRL-right arrow to advance through a text area, the caret skips past the first word of the next line.
This also happens in regular HTML when you extend a selection (SHIFT-CTRL-right arrow), it selects the word at the end of the line *and* the first word of the next line.


Reproducible: Always

Steps to Reproduce:
To see caret problem in textarea,
1.  Enter several lines of text
2.  Click in first row and repeatedly press CTRL-right arrow.

To see selection problem in regular HTML:
1.  In a bugzilla bug report, double-click a word in the first line of a comment, like "regular" in the line above.
2.  Press SHIFT-CTRL-right arrow repeatedly.
Actual Results:  
When the caret or selection end is on the last word of the line, it jumps *PAST* the first word on the next line.

Expected Results:  
The caret should advance to the start of the next line; the selection end should advance to select the last word of the current line and any whitespace.  That's what happens in Thunderbird 2 and Windows Notepad.

I guessed GFX:Thebes as the component for this bug since it's sort of like bug 383542.  It doesn't seem like an Editor bug since it occurs selecting in HTML as well as in Textarea.

Comment 1

[Samuel Sidler (old account; do not CC)]

I can confirm this using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007080815 Minefield/3.0a8pre.

The STR are exactly as listed in comment 0. Searching for dupes...

Comment 2

[Samuel Sidler (old account; do not CC)]

Didn't see any dupes, confirming. Requesting blocking for 1.9. I'm not sure this is the right component, but I'll leave it here per comment 0 and someone else can move it to a more appropriate one (Core::Selection maybe?).

Comment 3

[Uri Bernstein]

CC'ing roc.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The changes in bug 389421 might affect this. I don't have a build to test right now.

Comment 5

[Uri Bernstein]

This is not fixed by the patch on bug 389421.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

I don't know who reviews this code these days other than me ... picking mrbkap as a super-reviewer, and Simon as a reviewer. Uri, if you could look at it too, that'd be great.

The idea should be completely clear from the comment in nsFrame.

I figured out why things were acting weird in my general word-movement tests with eat_space_next_to_word set to true, so I added those tests as well as specific tests for this bug. I'm adding tests to the same mochitest file because I don't want to clone all its infrastructure.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 285027 [details] [diff] [review]
fix

sr=mrbkap

Comment 8

[Uri Bernstein]

As far as I can see, the patch doesn't accomplish the desired behavior: The caret first moves to after the space at the end of the line (instead of the beginning of the next line), and then, pressing option-right again, moves to the end of the first word on the next line (although it should only stop at beginnings of words).

The root cause of the bug is, I think, the issue referred to in [1]. The beginning of the "next" line, being a beginning of a frame, is not considered a valid place to stop. I'll try to see if I can work around this for this case.

[1] http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsTextFrameThebes.cpp&rev=3.115&mark=4578-4582#4548

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Actually on Mac at least the platform behaviour seems to be "stop at the end of the current line *and* at the start of the next line".

By stopping at the end of the line at least we don't skip over two words, just one. I thought that was the most important thing.

Comment 10

[Uri Bernstein]

(In reply to comment #9)
> Actually on Mac at least the platform behaviour seems to be "stop at the end of
> the current line *and* at the start of the next line".

That's not what I'm seeing, neither in Safari nor in TextEdit. Using option-right, the caret first stops at the end of the last word on the line, then at the end of the first word on the next line.
Windows is the same, with s/end/beginning/.

Comment 11

[Uri Bernstein]

Attachment: alt fix

This attempts to work around the fact that cIter.HaveWordBreakBefore() returns PR_FALSE at beginnings of lines.
As far as I can tell, it restores the original behavior.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Huh, so Textwrangler is different/wrong.

Probably a better fix would be to make cIter.HaveWordBreakBefore() return PR_TRUE at the beginning of the line. I should try that. If that doesn't work out or is hard, we'll do it your way instead.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I want 391584 to land before I work on this.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Actually I think 391584 completely fixed this on trunk. Anyone disagree?

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Well, I'm sure 391584 fixed this bug as filed. If there are any more bugs in this area, please file them.

Comment 16

[Jeff Walden [:Waldo]]

We should make sure the particular situation described here is tested in Mochitests.

Comment 17

[(no longer active)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050810 Minefield/3.0pre

Verified.