Closed Bug 244392 Opened 20 years ago Closed 20 years ago

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

Categories

(Core :: DOM: Editor, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jesales, Assigned: peterv)

References

()

Details

(Keywords: fixed-aviary1.0, fixed1.7.5, regression)

Attachments

(1 file, 2 obsolete files)

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.
(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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows ME → All
Hardware: PC → All
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>
Severity: normal → critical
Flags: blocking1.8a3?
Flags: blocking1.7.3?
Keywords: regression
Summary: Removing lines form a Bullet list using the [ back arrow ] or [Delete] keys results in <> and </> tags inbeded in code when in Normal Mode. → Removing lines form a Bullet list using the [ back arrow ] or [Delete] keys results in <> and </> tags embedded in code when in Normal Mode
*** Bug 247276 has been marked as a duplicate of this bug. ***
in addition to blocking 1.7.3 and 1.8a3, this would be great to fix before the
next version of thunderbird.
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0?
fix component (since this affects mail)
Assignee: composer → mozeditor
Component: Editor: Composer → Editor: Core
QA Contact: bugzilla
this regressed between linux trunk 2004020908 and 2004021008, but nothing in
that window looks like a likely culprit.
cc some people who may have introduced this regression (over 6 months ago)
backing things out of CVS indicates this is from bug 16603
reassign to peterv since he regressed (per comment 10)
Assignee: mozeditor → peterv
Status: NEW → ASSIGNED
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.
Attached patch v1 (obsolete) — Splinter Review
Attachment #156361 - Flags: superreview?(jst)
Attachment #156361 - Flags: review?(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 "".
Attachment #156361 - Flags: review?(brade) → review+
Yep, the empty-string-atom is a normal atom just like any other.
I'm not sure I like the change to nsHTMLDocument.cpp. Isn't there a risk we'll
hide bugs that way?
Flags: blocking1.8a3? → blocking1.8a3-
note, the recently released Netscape 7.2 has this bug.
(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?
doh! I read that wrong. You might want to #ifdef DEBUG it though..
Attached patch v1.1 (obsolete) — Splinter Review
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.
Attachment #156361 - Attachment is obsolete: true
Attached patch v1.1Splinter Review
Bah.
Attachment #156414 - Attachment is obsolete: true
Attachment #156361 - Flags: superreview?(jst)
Comment on attachment 156415 [details] [diff] [review]
v1.1

Carry forward r=brade.
Attachment #156415 - Flags: superreview?(jst)
Attachment #156415 - Flags: review+
(In reply to comment #19)
> doh! I read that wrong. You might want to #ifdef DEBUG it though..

The whole method is #ifdef DEBUG :-).
The new code in nsHTMLDocument.cpp still isn't debug only? (we won't actually do
anything in optimized, other then waste cycles)
Comment on attachment 156415 [details] [diff] [review]
v1.1

sr=jst
Attachment #156415 - Flags: superreview?(jst) → superreview+
Fixed on the trunk, requesting approvals for branches, as this is a trivial fix
that could fix odd crashes for people...
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
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 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.
Attachment #156415 - Flags: approval1.7.3?
Attachment #156415 - Flags: approval-aviary?
(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.
(In reply to comment #29)
> Trivial fix to avoid crashes in editor.

Ugh, not crashes but weird behaviour and invalid documents.
Comment on attachment 156415 [details] [diff] [review]
v1.1

a=mkaply for 1.7 and aviary
Attachment #156415 - Flags: approval1.7.3?
Attachment #156415 - Flags: approval1.7.3+
Attachment #156415 - Flags: approval-aviary?
Attachment #156415 - Flags: approval-aviary+
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.
(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.
Checked in on 1.7 and aviary branches.
Flags: blocking1.7.3?
Flags: blocking-aviary1.0PR?
Flags: blocking-aviary1.0?
*** Bug 257519 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: