102220 - caret loc. wrong after up/down arrow, edit, up/down arrow

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Brent]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.4) Gecko/20010913
BuildID:    2001091303

Happens while editing text in a box such as the one used to type this,
or while composing a mail message.  If the user hits up or down arrow, then
inserts or deletes text, then hits up or down arrow again, the caret will be
positioned as if no insertion or deletion had occurred.
If left or right arrow are used before or after the edit, then the caret will be
in the right place after up or down arrow.

Reproducible: Always
Steps to Reproduce:
(loc. = location)
1. Type some text.  These 5 words, one per line, will do:
the
quick
brown
fox
jumped
2. Loc. of caret is after the 'd'.  Hit left arrow 5 times.
3. Loc. of caret is between 'j' and 'u'.  Hit up arrow.
4. Loc. of caret is between 'f' and 'o'.  Type "1" and hit up arrow.

Actual Results:  Loc. of caret is between 'b' and 'r'.

Expected Results:  Loc. of caret should be between 'r' and 'o'.

Variation 1:
Step 4. Loc of caret is between 'f' and 'o'.  Hit backspace.  The 'f' is
deleted.  The caret is in front of "ox".  Hit up arrow.  Loc. of caret is
between 'b' and 'r'.  Loc. of caret should be in front of the 'b'.

Variation 2:
Step 2. Loc. of caret is after the 'd'.  Hit up arrow.
Step 3. Loc. of caret is after the 'x'.  Hit left arrow 2 times.
Now both versions of step 4 work correctly.

This also happens with the down arrow key.

If the left or right arrow key is hit between the 2 up or down arrow key hits,
then the caret moves to the correct loc.

Since the delete key does not change the caret's loc, it appears to work correctly.

Comment 1

[kinmoz]

--> [e-mail] (selection navigation)

Comment 2

[Alex Vincent [:WeirdAl]]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.5+) Gecko/20011108

For me, caret ended up left of "b" in brown.

Comment 3

[anthonyd]

measure text if off. marking EDITORBASE and Future.

Comment 4

[Syd Logan]

marking EDITORBASE+

Comment 5

[Shotaro Kamio]

The reason of this bug is that
 "nsSelection" holds old "mDesiredX" while the caret moved.
"nsSelection" needs to do "InvalidateDesiredX()" when chars are inserted to.
However, there is no way for "nsSelection" to aware of the insertion.

So, we need to add some functions to notify "nsSelection" of the insertion(and
the deletion).
I wrote a patch as an example of this.

Comment 6

[Shotaro Kamio]

Attachment: an example of the patch

Try this patch.
This patch also fixes the same bug in text mail composition.

Comment 7

[saari (gone)]

Mike, try to test this patch and get it into 0.9.9

Comment 8

[Shotaro Kamio]

*** Bug 124837 has been marked as a duplicate of this bug. ***

Comment 9

[mjudge]

the patch puts too much knowlege of insertion ect into selection.  I have a fix 
I am posting that will simply invalidate the desiredX when ::Collapse is 
called.  Collapse is called anytime the cursor is moved.  I think the is a 
simpler solution.

Comment 10

[mjudge]

Attachment: simple fix to invalidate the X coord when collapsing caret ever.

simpler fix i think for ::Collapse to fix this bug

Comment 11

[Simon Fraser [no longer active]]

Comment on attachment 76637 [details] [diff] [review]
simple fix to invalidate the X coord when collapsing caret ever.

sr=sfraser

Comment 12

[Kathleen :Brade]

Comment on attachment 76637 [details] [diff] [review]
simple fix to invalidate the X coord when collapsing caret ever.

r=brade

Comment 13

[Shotaro Kamio]

mjudge, I found an unwanted behavior in your patch.

Steps to reproduce:
1. Type 3 lines like below.
abcde[Enter]
[Enter]
fghij[Enter]
2. Place the caret at first line. (e.g. At after "c")
3. Hit Down arrow twice.

After 3., the caret is in front of "f".
We expect the caret is at after "h" as other editor apps do.

So, we should InvalidateDesiredX() only when edited.

Comment 14

[mjudge]

yes i know about this behaviour and I have allready fixed it in my tree.  The 
problem was when we did not use collapse but instead were calling collapse from 
inside nsSelection.  

Comment 15

[mjudge]

I thought i could just fix this on my own but here is the new patch. I will now 
go and get an r and an sr again.

Comment 16

[mjudge]

Attachment: fix to maintain desired x setting when collapse is called internally

basically the same patch but with a temp variable to remember old value.

Comment 17

[kinmoz]

Comment on attachment 77341 [details] [diff] [review]
fix to maintain desired x setting when collapse is called internally

sr=[e-mail]

Can we add a little comment to the code about why we need to save and restore
mDesiredXSet?

Comment 18

[Scott Collins]

Comment on attachment 77341 [details] [diff] [review]
fix to maintain desired x setting when collapse is called internally

a=scc

Comment 19

[scottputterman]

adding adt1.0.0 nomination

Comment 20

[Jaime Rodriguez, Jr.]

Pls check this into the trunk for a couple of days, and have QA verify that the
patch resolves the issue, and does not cause any regression.

Comment 21

[Jaime Rodriguez, Jr.]

Sujay/Patty - Can you take a look at this, once it lands on the trunk?

Comment 22

[Brent]

Another test, using the shift key:
1. Type in:
abc
def
ghi
2. Move caret between 'd' and 'e'.
3. Hit <Shift-Right Arrow> to select 'e'.
4. Hit Up Arrow.  Caret is between 'd' and 'e', 'e' is no longer selected.
5. Hit Up Arrow again.  Caret is between 'b' and 'c'.
   Caret should be between 'a' and 'b'
----
1. Same as above.
2. Move caret bewteen 'e' and 'f'
3. Hit <Shift-Left Arrow> to select 'e'.
4. Hit Down Arrow.  Caret is between 'e' and 'f', 'e' is no longer selected.
5. Hit Down Arrow again.  Caret is between 'g' and 'h'.
   Caret should be between 'h' and 'i'.

This happened with build 2002041206 on Windows 2000.

There are other problems with caret movement after selection with shift arrow
keys.  Tried looking at Notepad and Wordpad (Windows 2000) for examples of
correct behaviour, but while Notepad seemed ok, Wordpad was not.

Comment 23

[mjudge]

this patch fixes those issues.

Comment 24

[saari (gone)]

cool, check it in to trunk so we can bake it and get it into 1.0

Comment 25

[mjudge]

checked into trunk sujay THIS time its yours ;-) go to it man!

Comment 26

[scottputterman]

adding adt1.0.0-.  Let's keep this on the trunk and then see if we should take
it for rtm.  If this is needs to land in Mozilla1.0, please land it after Mach V
branches for Beta.

Comment 27

[sujay]

verified fix in 4/18 trunk...looks good...problem is gone.

Comment 28

[mjudge]

ok the truth is I accidentally checked this in last night.  I swear it was an 
accident.  I read that sujay had verified on trunk and thought that was a green 
light. now that I was trying to update the bug that I had fixed this, I saw the 
previous comment.

I am a little puzzled why this would be pushed back.  Its a very simple and 
localized patch.  Anyway I am marking fixed. If I should back this out of the 
branch reopen and lemme know.

Comment 29

[sujay]

verified in 4/22 branch build.