Closed
Bug 56880
Opened 24 years ago
Closed 24 years ago
backspace can delete entire contents of style node
Categories
(Core :: DOM: Editor, defect, P3)
Core
DOM: Editor
Tracking
()
VERIFIED
FIXED
mozilla0.8
People
(Reporter: akkzilla, Assigned: mozeditor)
Details
(Keywords: dataloss)
Attachments
(1 file)
4.56 KB,
patch
|
Details | Diff | Splinter Review |
Seen on trunk, 10/16. Run composer. Type <accel-i>Hello <accel-i>world (accel-i = hold down the accelerator modifier and i, to toggle italics) Type five backspaces; the letters of "world" progressively disappear. Type another backspace, expecting the space between Hello and world to disappear; instead, the whole word Hello is deleted.
Comment 1•24 years ago
|
||
I saw this on a Macintosh debug build from yesterday morning (pressing delete appropriately deleted text and then a whole link with one keypress).
OS: Linux → All
Hardware: PC → All
Comment 2•24 years ago
|
||
this is not good, i would think that would be considered dataloss, marking as such
Assignee | ||
Comment 4•24 years ago
|
||
i cannot reproduce this. Will mark WORKSFORME unless someone says they can reproduce it.
Reporter | ||
Comment 5•24 years ago
|
||
The problem seems gone in the current trunk (will try branch next).
Reporter | ||
Comment 6•24 years ago
|
||
Problem is still there on the branch.
Assignee | ||
Comment 7•24 years ago
|
||
hmm, i cannot reproduce it on my branch build.
Assignee | ||
Comment 8•24 years ago
|
||
i checked with Akkana: she didn't actually see this happen as described here. Rather, she saw some case she cant quite remember. If anyone finds a way to reproduce this, let me know.
Reporter | ||
Comment 9•24 years ago
|
||
Apparently it's gotten harder to reproduce. I can't reproduce it using the steps described in the bug; I saw it this morning when editing a mail message. (which means it probably still happens on the trunk, too.) I was in html mail compose, I copied a bunch of plaintext quotes one-by-one from another window, then selected them all and made them preformatted (I really wanted plaintext compose but wasn't able to get it from the browser window), then did a lot of editing of line ends to stick them all together and fix the line lengths (since they all ended up in separate pre's), then started adding comments between the various plaintext-quoted blocks. I saw the bug when I started deleting my own remarks back to the beginning of a section, overshot and deleted into a quoted block -- which deleted the whole quoted block. If I find a cookbook way of reproducing it, I will describe it here.
Assignee | ||
Comment 10•24 years ago
|
||
i found a way to get this to happen. use the original test case EXCEPT, left arrow once before deleting. investigating bug now...
Assignee | ||
Comment 11•24 years ago
|
||
fix in hand; attaching next
Whiteboard: [rtm need info] → [rtm need info] fix in hand; getting reviews...
Assignee | ||
Comment 12•24 years ago
|
||
Assignee | ||
Comment 13•24 years ago
|
||
the patch is simpler than it looks: basically, an if/elseif/else clause has been reordered. This has happenedin two places in the same routine, for two nearly identical if/elseif/else clauses. One is for backspace, the other forward delete. We now test for text nodes *before* checking for non-containers, since text nodes are not considered containers by the dtd.
Assignee | ||
Comment 14•24 years ago
|
||
ccing simon
Comment 15•24 years ago
|
||
The patch looks good. As long as there is some serious bashing on this (lots of typing testing), sr=sfraser
Reporter | ||
Comment 16•24 years ago
|
||
The change is very straightforward, makes sense, and does indeed fix the problem. r=akkana.
Comment 17•24 years ago
|
||
I'm not sure if this is right or not, could someone test deleting around lists and tables? I'm seeing some funkiness which may or may not be related to this patch.
Assignee | ||
Comment 18•24 years ago
|
||
i haven't found any differences caused by the patch. There are some unrelated oddities that are less serious and that are independent of this patch.
Comment 19•24 years ago
|
||
yes, Joe is right; the problems I see are unrelated to this patch. I have filed a different bug on those issues. Setting to rtm+ for Joe.
Whiteboard: [rtm need info] fix in hand; getting reviews... → [rtm+] reviewed fix in hand
Comment 20•24 years ago
|
||
PDT marking [rtm need info]. Does this only happen if you use the accelerators, or is there a simpler way to see this? Since the patch is large-ish for this state of the branch, please check into the trunk and get help from editor QA to verify that there aren't any regressions. After that, please mark rtm+ again for consideration.
Whiteboard: [rtm+] reviewed fix in hand → [rtm need info] reviewed fix in hand
Comment 21•24 years ago
|
||
Joe, push this through getting into the trunk, and pass on the branch.
Comment 22•24 years ago
|
||
beppe: Does your comment mean that this should fall off of the rtm radar?
Comment 23•24 years ago
|
||
PDT marking [rtm-] since that seems to be what beppe intended.
Whiteboard: [rtm need info] reviewed fix in hand → [rtm-] reviewed fix in hand
Assignee | ||
Comment 25•24 years ago
|
||
this will definitely land for 0.8
Target Milestone: mozilla0.9 → mozilla0.8
Assignee | ||
Comment 26•24 years ago
|
||
fixed
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Keywords: rtm
Resolution: --- → FIXED
Whiteboard: [rtm-] reviewed fix in hand
Comment 27•23 years ago
|
||
this problem is fixed. however a new problem is discovered. filing separate bug
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•