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

VERIFIED FIXED in mozilla1.0

Status

()

Core
Editor
--
major
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Brent, Assigned: mjudge)

Tracking

({topembed+})

Trunk
mozilla1.0
x86
Windows 98
topembed+
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Hixie-P0][Hixie-1.0][caret] EDITORBASE+ [adt2], URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

16 years ago
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

16 years ago
--> mjudge@netscape.com (selection navigation)
Assignee: kin → mjudge
Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.5+) Gecko/20011108

For me, caret ended up left of "b" in brown.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [Hixie-P0][Hixie-1.0]

Comment 3

16 years ago
measure text if off. marking EDITORBASE and Future.
Status: NEW → ASSIGNED
Whiteboard: [Hixie-P0][Hixie-1.0] → EDITORBASE[Hixie-P0][Hixie-1.0]
Target Milestone: --- → Future

Comment 4

16 years ago
marking EDITORBASE+
Whiteboard: EDITORBASE[Hixie-P0][Hixie-1.0] → EDITORBASE+[Hixie-P0][Hixie-1.0]

Comment 5

16 years ago
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

16 years ago
Created attachment 66880 [details] [diff] [review]
an example of the patch

Try this patch.
This patch also fixes the same bug in text mail composition.
Whiteboard: EDITORBASE+[Hixie-P0][Hixie-1.0] → [Hixie-P0][Hixie-1.0][caret] EDITORBASE+
Severity: normal → major
Keywords: mozilla1.0

Updated

16 years ago
Keywords: nsbeta1+

Comment 7

16 years ago
Mike, try to test this patch and get it into 0.9.9
Target Milestone: Future → mozilla0.9.9

Updated

16 years ago
Keywords: topembed+

Comment 8

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

Updated

16 years ago
Target Milestone: mozilla0.9.9 → mozilla1.0
(Assignee)

Comment 9

16 years ago
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.
(Assignee)

Comment 10

16 years ago
Created attachment 76637 [details] [diff] [review]
simple fix to invalidate the X coord when collapsing caret ever.

simpler fix i think for ::Collapse to fix this bug
Attachment #66880 - Attachment is obsolete: true

Comment 11

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

sr=sfraser
Attachment #76637 - Flags: superreview+

Comment 12

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

r=brade
Attachment #76637 - Flags: review+

Updated

16 years ago
Whiteboard: [Hixie-P0][Hixie-1.0][caret] EDITORBASE+ → [Hixie-P0][Hixie-1.0][caret] EDITORBASE+ [adt2]

Comment 13

16 years ago
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.
(Assignee)

Comment 14

16 years ago
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.  
(Assignee)

Comment 15

16 years ago
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.
(Assignee)

Comment 16

16 years ago
Created attachment 77341 [details] [diff] [review]
fix to maintain desired x setting when collapse is called internally

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

Updated

16 years ago
Attachment #77341 - Flags: review+

Comment 17

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

sr=kin@netscape.com

Can we add a little comment to the code about why we need to save and restore
mDesiredXSet?
Attachment #77341 - Flags: superreview+

Updated

16 years ago
Attachment #76637 - Attachment is obsolete: true

Comment 18

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

a=scc
Attachment #77341 - Flags: approval+

Comment 19

16 years ago
adding adt1.0.0 nomination
Keywords: adt1.0.0

Comment 20

16 years ago
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

16 years ago
Sujay/Patty - Can you take a look at this, once it lands on the trunk?
(Reporter)

Comment 22

16 years ago
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.
(Assignee)

Comment 23

16 years ago
this patch fixes those issues.

Comment 24

16 years ago
cool, check it in to trunk so we can bake it and get it into 1.0
(Assignee)

Comment 25

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

Comment 26

16 years ago
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.
Keywords: adt1.0.0 → adt1.0.0-

Comment 27

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

(Assignee)

Comment 28

16 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Keywords: fixed1.0.0
Resolution: --- → FIXED

Comment 29

16 years ago
verified in 4/22 branch build.
Status: RESOLVED → VERIFIED
Keywords: verified1.0.0
You need to log in before you can comment on or make changes to this bug.