Closed Bug 334608 Opened 18 years ago Closed 18 years ago

Deleting the characters with the delete key confuses the caret

Categories

(Core :: DOM: Editor, defect, P2)

x86
All
defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: mrbkap, Assigned: mrbkap)

References

()

Details

(Keywords: regression, Whiteboard: [patch])

Attachments

(2 files)

I noticed this on a nightly build:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060418 Firefox/3.0a1

Steps to reproduce:
 * Go to the url (or any url with a text field).
 * Type some text, I'll use '123456'
 * Position the caret between the 3 and 4.
 * While the caret is in its "on" state (i.e., it is visible) press 'delete'
 * Note the caret turd at the beginning (!) of the input box

I cannot seem to reproduce this by deleting characters with the backspace key.
Blocks: 287813
No longer blocks: 281813
OS: Windows XP → All
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [patch]
Attached patch Potential patchSplinter Review
So, this patch fixes this bug for me. I don't believe the StCaretHider is necessary here anymore, though I'm not entirely sure what it's doing here in the first place. Why do we not want the caret displaying at this point in time? Did it leave turds? Anyway, all of the invalidation and such should be handled by the presshell and regular frame painting paths now.

I'm not sure who should review this... I'll pick on brade and roc for now, though...
Attachment #219410 - Flags: superreview?(roc)
Attachment #219410 - Flags: review?(brade)
By the way, the problem seems to be that when the StCaretHider goes away, the call to SetCaretVisible(true) causes the caret to calculate a bogus offset (0, 0) for its position in the current frame.
Attached patch AlternativeSplinter Review
This patch is good with or without the other patch. The reason the StCaretHider is confusing things is that we're caching a (0, 0) frame offset when we erase the caret, so when the caret tries to blink at the same position, we draw it at the cached, wrong offset instead of the right one.
Attachment #219476 - Flags: superreview?(roc)
Attachment #219476 - Flags: review?(roc)
Attachment #219410 - Flags: superreview?(roc) → superreview+
Attachment #219476 - Flags: superreview?(roc)
Attachment #219476 - Flags: superreview+
Attachment #219476 - Flags: review?(roc)
Attachment #219476 - Flags: review+
I checked attachment 219476 [details] [diff] [review] into the trunk. Leaving the bug open for attachment 219410 [details] [diff] [review].
I would feel better if Simon had a chance to look at the first (small) patch in this bug.
Either patch looks fine.
Attachment #219410 - Flags: review?(brade) → review+
Both fixes have now been checked into the trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Depends on: 335663
I backed out attachment 219410 [details] [diff] [review] because it caused more problems than it solved (it caused 2 problems, and solved 0 ;-)).
I went ballistic testing combinations of backspace and delete operation with and without the caret blinking, and this is Verified FIXED using build 2006-04-28-05 of SeaMonkey trunk on Windows XP.
Status: RESOLVED → VERIFIED
QA Contact: editor
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: