backspace can delete entire contents of style node

VERIFIED FIXED in mozilla0.8

Status

()

Core
Editor
P3
major
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Akkana Peck, Assigned: Joe Francis)

Tracking

({dataloss})

Trunk
mozilla0.8
dataloss
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

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

17 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

17 years ago
this is not good, i would think that would be considered dataloss, marking as such
Keywords: dataloss, rtm
Whiteboard: [rtm need info]
Target Milestone: --- → M19
(Assignee)

Comment 3

17 years ago
hummina hummina hummina
Status: NEW → ASSIGNED
(Assignee)

Comment 4

17 years ago
i cannot reproduce this.  Will mark WORKSFORME unless someone says they can 
reproduce it.
(Reporter)

Comment 5

17 years ago
The problem seems gone in the current trunk (will try branch next).
(Reporter)

Comment 6

17 years ago
Problem is still there on the branch.
(Assignee)

Comment 7

17 years ago
hmm, i cannot reproduce it on my branch build.
(Assignee)

Comment 8

17 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

17 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

17 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

17 years ago
fix in hand; attaching next
Whiteboard: [rtm need info] → [rtm need info] fix in hand; getting reviews...
(Assignee)

Comment 12

17 years ago
Created attachment 18071 [details] [diff] [review]
patch for editor/base/nsHTMLEditRules.cpp
(Assignee)

Comment 13

17 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

17 years ago
ccing simon

Comment 15

17 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

17 years ago
The change is very straightforward, makes sense, and does indeed fix the
problem. r=akkana.

Comment 17

17 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

17 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

17 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

17 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

17 years ago
Joe, push this through getting into the trunk, and pass on the branch.

Comment 22

17 years ago
beppe: Does your comment mean that this should fall off of the rtm radar?

Comment 23

17 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 24

17 years ago
moz 0.9
Target Milestone: M19 → mozilla0.9
(Assignee)

Comment 25

17 years ago
this will definitely land for 0.8
Target Milestone: mozilla0.9 → mozilla0.8
(Assignee)

Comment 26

17 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Keywords: rtm
Resolution: --- → FIXED
Whiteboard: [rtm-] reviewed fix in hand

Comment 27

17 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.