Closed Bug 46209 Opened 22 years ago Closed 21 years ago

can't delete leftover list item after cutting the rest

Categories

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

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: ekrock, Assigned: mozeditor)

References

Details

(Keywords: testcase, Whiteboard: [behavior][dogfood-] fix in hand, reviewed, a=asa)

Attachments

(2 files)

Using commercial 2000072108 on WinNT 4.0 SP4.

To repro, open attached test case file in Composer. Note the "1." item in the
middle of the page. Try to place the caret in front of it. You can't.

The document was left in this state after a cut operation in which I'd selected
all the items of a numbered list and cut them. I've seen the same
problem/behavior with bullet lists as well.
Nom. dogfood because this prevents me from using Composer to edit documents with
lists; I have to use a text editor or Nav4 Composer to get rid of the extra
bullet. Nom. nsbeta3 for correctness as a fallback if not accepted for dogfood.
Attached file test case
Keywords: testcase
assigning to jfrancis
Assignee: beppe → jfrancis
Target Milestone: --- → M18
Putting on [dogfood+] radar. 
Whiteboard: [dogfood+]
First, there is a workaround in this case -- put the caret at the start of the 
paragraph after the list item, and backspace several times; this will first take 
you into the list item, then remove it. Granted, this is not obvious.

We seem to get into this state when we read in a file with an <li> </li> which 
has a single space in. We fail, in this case, to add a <br> under the list item, 
which makes it editable.
Adding myself to the Cc list.
This is actually a dup of 22227.  Both table vells with a single space and list 
items with a single space cause me problems.  I don't have a way to know that 
they didn't render: I don't have a way to ask layout about whether some arbitrary 
content actually rendered.  Mike Judge is working on a way for the editor to be 
able to ask layout that question.  Once that's done I should be able to use that 
to fix these two bugs.

Status: NEW → ASSIGNED
Per today's Seamonkey Mgrs Beta2 triage, keep on dogfood+, but wil not hold 
beta 2 for this.
Keywords: relnote2
Whiteboard: [dogfood+] → [dogfood+][nsbeta2-] NO HOLD FOR BETA2!!
fixed; thanks to mjudge for doing the real work here...
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
We had to back out the fix for this since it was causing data loss for
operations like bold.

The frame data the fix depends on is not valid sometimes because it needs to be
reflowed. In the case of the bold data loss, CheckVisibility() was returning
PR_FALSE for visible because the frame it was looking at had a zero offset and
zero length.

Reopening bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Adding ETA per daily PR2 mtg today.
Whiteboard: [dogfood+][nsbeta2-] NO HOLD FOR BETA2!! → [dogfood+][nsbeta2-]ETA 7/26 Trunk - 7/27 Branch
Here's the deal:  I have a fix for 22227 and for a bug very much like this one, 
but not quite this bug.  Basically, I can detect td's and li's that have non-
rendering whitespace, and put in a <br> so that you will be able to click in 
these elemetns and actually get a caret.  BUT I CAN ONLY DO THAT AT DOCUMENT LOAD 
TIME.  I can't yet detect the situation "on the fly", ie, caused by edits of a 
document.  This bug was found by ekrock editting on the fly, rather than loading 
a document that already had an <li> with whitespace in it.  

Beth has told me to check this in.  It will fix 22227 and it will reduce the 
number of scenarios where this bug bites us.  But it will not actually fix the 
specific scenario ekrock described, so I can't close this bug.

Really fixing these whitespace issues is a challenge, and attempting to do it 
right at the end of beta2 is a mistake.  This should be one of a select group of 
editor problems that we tackle right from the beginning of beta3 and have time to 
get right.  

I recommend that we mark this nsbeta3+.
sujay,

to test what i've done, use this document:

<html><body><ul><li> </li></ul></body></html>

Open that document in composer.  Before, you would get a list item that you 
couldn't click in.  Now, you should be able to click in the list item, and type, 
etc...
Moving to [dogfood-].
Whiteboard: [dogfood+][nsbeta2-]ETA 7/26 Trunk - 7/27 Branch → [dogfood-][nsbeta2-]ETA 7/26 Trunk - 7/27 Branch
Putting on [nsbeta3+] radar.
Whiteboard: [dogfood-][nsbeta2-]ETA 7/26 Trunk - 7/27 Branch → [dogfood-][nsbeta2-][nsbeta3+]
dup of 47015


*** This bug has been marked as a duplicate of 47015 ***
Status: REOPENED → RESOLVED
Closed: 22 years ago22 years ago
Resolution: --- → DUPLICATE
verified in 7/31 build.
Status: RESOLVED → VERIFIED
Adding keyword to bugs which already show a nsbeta2 triage value in the status 
whiteboard so the queries don't get screwed up.
Keywords: nsbeta2
reopening (unduping) this bug
Status: VERIFIED → REOPENED
Resolution: DUPLICATE → ---
moz 092
Target Milestone: M18 → mozilla0.9.2
Whiteboard: [dogfood-][nsbeta2-][nsbeta3+] → [behavior][dogfood-][nsbeta2-][nsbeta3+]
Keywords: nsbeta2, nsbeta3
Whiteboard: [behavior][dogfood-][nsbeta2-][nsbeta3+] → [behavior][dogfood-]
layout tells me that the textnode inside the li has a frame.  I am relying on 
formatting-only whitespace nodes toot have frames.  That is how I tell if there 
is something in the li.  If there is nothing, I put in a br so that the user will 
have somewhere to click and get a caret.

I don't know why layout is telling me there is a frame.  

Kin, do you know?
I have attached a patch to fix this.  I had to get a bit tricky inside
IsEditable().  It used to just equate editability with having a frame.  Now it:
1) still does that for everything but text content
2) for text content without a frame, returns false
3) for text content with a frame that is dirty, returns true (assumes dirty text
frames are editable)
4) for text content with non-dirty frames, checks their width.  Editablility
corresponds to non-zero width.

Removing any of these cases causes some very bad shit.
Whiteboard: [behavior][dogfood-] → [behavior][dogfood-] fix in hand
+    if (rect.width) 
Would prefer
+    if (rect.width > 0) 
just to make it obvious that you didn't botch or make a typo and meant to compare 
a pointer to null.

otherwise, r/sr=sfraser
Whiteboard: [behavior][dogfood-] fix in hand → [behavior][dogfood-] fix in hand, need a=
Blocks: 83989
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Whiteboard: [behavior][dogfood-] fix in hand, need a= → [behavior][dogfood-] fix in hand, reviewed, a=asa
fix checked in
Status: REOPENED → RESOLVED
Closed: 22 years ago21 years ago
Resolution: --- → FIXED
verified win2k 46209

(I could delete the list item with backspace, but I still couldn't put the
cursor in front of it)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.