244392 - Removing lines form a Bullet list using the [ back arrow ] or [Delete] keys results in <> and </> tags embedded in code when in Normal Mode

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jack Sales]

User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8a) Gecko/20040515
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8a) Gecko/20040515

Removing lines form a Bullet list using the [ back arrow ] or [Delete] keys
results in <> and </> tags inbeded in code when in Normal Mode.   When page is
saved and opened all the "<" and ">" are replaced with &lt; and &gt;.
Whan the error accures all the (related) <li> and </li> tags are replaced with
<> and </>.
This happens when you are making changes to the structure of a list.
This may also happen when using cut and paste.

Reproducible: Always
Steps to Reproduce:
1.crate a four line list
2.place curser at the end of line 3
3.press [Delete] key or [Backspace]


Actual Results:  
lines run together (some time more than one).
<> and </> tags inbeded in code and you do not see them in normal edit mode.
When you view the page in a browser window you find <> <> in your page.
Thay are converted to &lt; and &gt; in the code.

Expected Results:  
delete the line or <br> but not the <li> </li> tags in more that one line.

see http://www.darksky.org/mozilla-errors.html for more.

Comment 1

[Tyson]

(In reply to comment #0)


When typing in an editable document.
I get blank/empty tags when there are two like tags next to each other, and I
press delete while i'm between the two tags. It looks like this:

<>this was a div tag before delete was pressed</>


Here is an example:

I have an editable document in an iframe for a rtf editor.  If I have content in
the editable document like so:


Html - User View
------------------------------------------------------------------------------
The next word is formatted. This is not formatted but in the same h1.
This is in a new h1 and has a class applied to it.
------------------------------------------------------------------------------

Html - Code View
 ------------------------------------------------------------------------------
<h1 class="style1">The next word is <span
style="font-weight:bold;">formatted</span>. This is not formatted but in the
same h1.</h1><h1 class="style2">This is in a new h1 and has a class applied to
it.</h1>
-------------------------------------------------------------------------------


In the user view if you go to the end of a line with the cursor:

"The next word is formatted. This is not formatted but in the same h1.|cursor|"

And press delete mozilla's editor does this:

Html - Code View
 ------------------------------------------------------------------------------
<>The next word is <span style="font-weight:bold;">formatted</span>. This is not
formatted but in the same h1.This is in a new h1 and has a class applied to it.</>
-------------------------------------------------------------------------------

Once this has happened, I have to then check from the current node back up to
the body tag to makes sure the mozilla editor hasn't left <> blank tags behind it.

Is this the way the mozilla is suppose to function when joining like tags in an
editable document?

As I have had to work around these problems there are a few things that appear
to cause this problem.

If there are \r\n control characters at the end of the innerHTML of the first
node that I am trying to join with a second.

If there are \r\n control characters between the too tags I am trying to join.

If those control characters are not there and the user presses delete or
backspace the two nodes usually join fine.

However there are some cases I have not covered yet, it appears spaces at the
end of a tag might cause it, also spaces between tags could as well.

Comment 2

[Kathleen :Brade]

I see this in mozilla 1.7 on Mac OSX.  It is VERY ugly/bad.  I'd like to know
when it regressed.

I can reproduce with these steps:
1. Create new html mail compose window
2. click bullet icon
3. type:  a[return]b[return]c[return]d
   you should now have full bullets with a letter next to each
4. press the left arrow so the caret is blinking before the 'd'
5. press backspace
   notice the bullet before the c is gone
6. select all
7. from insert menu, choose "HTML..."
Here is the resulting html:
<ul>
  <li>a</li>
  <li>b</li>
  <>cd<br>
  </>
</ul>

Comment 3

[timeless]

*** Bug 247276 has been marked as a duplicate of this bug. ***

Comment 4

[(not reading, please use [e-mail] instead)]

in addition to blocking 1.7.3 and 1.8a3, this would be great to fix before the
next version of thunderbird.

Comment 5

[Kathleen :Brade]

fix component (since this affects mail)

Comment 6

[Andrew Schultz]

this regressed between linux trunk 2004020908 and 2004021008, but nothing in
that window looks like a likely culprit.

Comment 7

[Asa Dotzler [:asa]]

Who can look into this? Time is pretty short on 1.8a3. Here's the hook for the
regression window.
http://bonsai.mozilla.org/cvsquery.cgi&branch=Head&dir=&file=&who=&sortby=Date&hours=2&date=explicit&mindate=02%2F09%2F2004+7%3A00&maxdate=02%2F10%2F2004+10%3A00

Comment 8

[Asa Dotzler [:asa]]

sorry for the bad link. try this one. 
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=Head&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=02%2F09%2F2004+7%3A00&maxdate=02%2F10%2F2004+10%3A00&cvsroot=%2Fcvsroot

Comment 9

[Kathleen :Brade]

cc some people who may have introduced this regression (over 6 months ago)

Comment 10

[Andrew Schultz]

backing things out of CVS indicates this is from bug 16603

Comment 11

[Kathleen :Brade]

reassign to peterv since he regressed (per comment 10)

Comment 12

[Peter Van der Beken [:peterv]]

Editor code is being silly and tries to create an element with an empty nodename
and I removed a IsEmpty check in bug 16603 since it really is the caller who
should check.
nsHTMLEditRules::JoinBlocks should probably not call
nsHTMLEditRules::ConvertListType with an empty aListType, but I'll let an editor
person sort that out (I'll file a new bug). Patch coming up that adds a check
for an empty name before calling CreateElem and adds some assertions to catch
other stuff like this.

Comment 13

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 14

[Kathleen :Brade]

Comment on attachment 156361 [details] [diff] [review]
v1

r=brade

One thing I didn't understand until just now is that do_GetAtom won't fail if
passed "".

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yep, the empty-string-atom is a normal atom just like any other.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm not sure I like the change to nsHTMLDocument.cpp. Isn't there a risk we'll
hide bugs that way?

Comment 17

[(not reading, please use [e-mail] instead)]

note, the recently released Netscape 7.2 has this bug.

Comment 18

[Peter Van der Beken [:peterv]]

(In reply to comment #16)
> I'm not sure I like the change to nsHTMLDocument.cpp. Isn't there a risk we'll
> hide bugs that way?

Adding an assertion will hide bugs?

Comment 19

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

doh! I read that wrong. You might want to #ifdef DEBUG it though..

Comment 20

[Peter Van der Beken [:peterv]]

Attachment: v1.1

I corrected the debug-only check in nsHTMLDocument::CreateElem to only check
for lowercase tagname if the document is XHTML *and* the element is in the
XHTML namespace. Minor change, I'll just transfer the r=brade to this one.

Comment 21

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Bah.

Comment 22

[Peter Van der Beken [:peterv]]

Comment on attachment 156415 [details] [diff] [review]
v1.1

Carry forward r=brade.

Comment 23

[Peter Van der Beken [:peterv]]

(In reply to comment #19)
> doh! I read that wrong. You might want to #ifdef DEBUG it though..

The whole method is #ifdef DEBUG :-).

Comment 24

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The new code in nsHTMLDocument.cpp still isn't debug only? (we won't actually do
anything in optimized, other then waste cycles)

Comment 25

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

double-doh!

Comment 26

[Johnny Stenback (:jst)]

Comment on attachment 156415 [details] [diff] [review]
v1.1

sr=jst

Comment 27

[Johnny Stenback (:jst)]

Fixed on the trunk, requesting approvals for branches, as this is a trivial fix
that could fix odd crashes for people...

Comment 28

[[e-mail]]

Damn, I forgot to CC myself, and duplicated all the debugging :-(
I determined that nsHTMLEditRules::JoinBlocks is trying to convert the list type
even if bMergeLists is false. This sounds wrong, as the new list type isn't set!

Comment 29

[Peter Van der Beken [:peterv]]

Comment on attachment 156415 [details] [diff] [review]
v1.1

Trivial fix to avoid crashes in editor. Most of the patch is DEBUG-only sanity
checking, the real fix is in the nsEditor.cpp chunk.

Comment 30

[Peter Van der Beken [:peterv]]

(In reply to comment #27)
> Fixed on the trunk, requesting approvals for branches, as this is a trivial fix
> that could fix odd crashes for people...

Er, Johnny, was this comment meant for a different bug? I don't see your checkin
so I'll check this in to trunk myself.

Comment 31

[Peter Van der Beken [:peterv]]

(In reply to comment #29)
> Trivial fix to avoid crashes in editor.

Ugh, not crashes but weird behaviour and invalid documents.

Comment 32

[Mike Kaply [:mkaply]]

Comment on attachment 156415 [details] [diff] [review]
v1.1

a=mkaply for 1.7 and aviary

Comment 33

[Scott MacGregor]

I didn't have much luck getting this particular patch to apply cleanly onto the
aviary branch. If you end up checking this into 1.7.3 can you post the patch you
use for that here? We might be able to re-use it for the aviary branch.

Comment 34

[Johnny Stenback (:jst)]

(In reply to comment #30)
> Er, Johnny, was this comment meant for a different bug? I don't see your checkin
> so I'll check this in to trunk myself.

Um, duh, yeah, that was meant for bug 198254.

Comment 35

[Peter Van der Beken [:peterv]]

Checked in on 1.7 and aviary branches.

Comment 36

[Asaf Romano (gone)]

*** Bug 257519 has been marked as a duplicate of this bug. ***