210110 - selection at end of line spans multiple lines in embedded text editor

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Mitch]

User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.5a) Gecko/20030611
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.5a) Gecko/20030611

When composing or forwarding a email, if you select the last word on
a line, the selection will span the last word *and* the first word on the
next line. This was definetly not happening on the previous build we have  -
i.e. 20030611 but is happening in todays cvs 20030620 (so i can't vouch for
intervening releases). Calum can you narrow please.

Reproducible: Always

Steps to Reproduce:
1. Compose an email with the following text as an example. Ensure the text
   starts at the begining of the line (i.e. column 0) in the composer.

The rain in spain falls
mainly on the plain.

2. Select the word "falls" in the text by double clicking your mouse
3.

Actual Results:  
Your selection will span 2 lines and select "falls" and "mainly" in the
example shown.

Expected Results:  
Selected only the word falls on the current line.

Comment 1

[Calum Mackay]

confirmed; Linux x86 20030626. I will try and see when this started.

Only happens when line ends in a newline; doesn't happen with word wrapped lines.

Also happens in *this* text widget I'm currently typing into, so has nothing to
do with mailnews. changing to Broswer

Comment 2

[Calum Mackay]

I've narrowed it down to between builds:

2003061707 OK
2003061807 BAD

where the build - and checkout - times are in BST (== GMT+1)

is this old enough to be in the 1.4 RC branch too? if so, should be a blocker?

Comment 3

[Calum Mackay]

I see a few editor:core checkins on the 17th: 183582, 194963 & 202037.

Comment 4

[Kathleen :Brade]

Does backing out bug 202046 fix this?

Comment 5

[Boris Zbarsky [:bzbarsky]]

This seems to be caused by the patch for bug 202046 -- backing that out locally
fixes the issue.

Comment 6

[Mitch]

Thanks Boris, Brade. I was just about to back out and recompile (i will anyhow
until the fix is checked in since it is very irritating when doing selections).

Comment 7

[Simon Montagu :smontagu]

Taking: I think this is caused by Bidi code.

Comment 8

[Simon Montagu :smontagu]

Attachment: work-around patch

This is a minimal patch which corrects this behaviour without regressing the
test cases attachment 120512 [details] and attachment 120513 [details] from bug 202406. It won't do
for checkin because it causes bad caret movement regressions in bidirectional
text, but for people who want a quick local fix it's better than just backing
out bug 2020406.

Comment 9

[Mitch]

Thanks Simon. Applied and works for me fine.

Comment 10

[Simon Montagu :smontagu]

In fact, the Bidi regressions aren't caused by my patch, they seem to be the
result of roc's last checkin to nsFrameTraversal.cpp.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsFrameTraversal.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/base/src&command=DIFF_FRAMESET&rev1=3.31&rev2=3.32

The very last change in that diff reverses the semantics of the test.

Comment 11

[Simon Montagu :smontagu]

Attachment: Improved patch

This version of the patch is probably more accurate, picking up a suggestion
from bz on IRC that we should be testing the mRect and mStateBits of the same
frame.

Of course, a better question would be "why does Bidi reordering create empty
frames at the end of lines, and can we make it stop?" :-)

Comment 12

[Simon Montagu :smontagu]

Fix checked in.

Comment 13

[Asa Dotzler [:asa]]

mozilla1.5a released. unsetting request.