Open Bug 54479 Opened 22 years ago Updated 2 years ago

execCommand("indent") unexpectedly directly nests list elements


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





(Reporter: ilmari, Unassigned)



(Keywords: html4, topembed-, Whiteboard: [QAHP-top] EDITORBASE- 5 days [t2])


(2 files)

When making nested lists in Mozilla, the resulting HTML does not validate.
The second level gets created outside the <li></l> tags, but it should be inside
the last <li></li> pair in the level above. See attached testcase for details
yes, we are aware of the issue and this is on the list to be resolved after rtm,
currently the generated output renders correctly in all browsers, but it is
still invlaid html.

i thought we had an open bug for this already, but I can't find it. setting this
to future, assigning to jfrancis, adding keywords, adjusting priority and
changing platform/os fields
Assignee: beppe → jfrancis
Keywords: correctness
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Future
Ever confirmed: true
neglected to set priority
Priority: P3 → P2
yea verily
Target Milestone: Future → mozilla0.9
this bug was FUTURE, but the keywords say "correctness" and "mozilla0.9"

changing Target milestone to mozilla0.9 set the keywords without explanation

moving back to future, removing mozilla0.9
Keywords: mozilla0.9
Target Milestone: mozilla0.9 → Future
I set it because it is a correctness bug and I don't think mozilla should go out
generating invalid HTML.  It even has a P2 priority.  I would have hoped that
was obvious.  

As far as I know it is improper to remove someone else's nomination.  If you
don't like it, ignore it, or ask them to change or remove it.
hen please in the future place some sort of text as to why you believe it should 
be nominated, don't just nominate and say nothing.
bringing into 1.0
Target Milestone: Future → mozilla1.0
there is a side issue from bug 68132, where correctly nested lists are not
outdented correctly.
Keywords: nsbeta1
Whiteboard: [QAHP]
Blocks: 104166
The output is semantically wrong, because the indented list belongs to the <li>
and is not an item on its own.

This is a highly visible case, where Mozilla generates invalid and inlogical
HTML. Mozilla should create correct (logically and syntactically) output and
promote standards. I think, this is a MUSTfix for 1.0.
Adding html4 mozilla1.0 keywords. I'd add correctness, but it's gone.
Keywords: html4, mozilla1.0
Whiteboard: [QAHP] → [QAHP] EDITORBASE
Taking ownership of this bug in order to help jfrancis who has BR's selection
on his shoulders... Setting 5 days for EDITORBASE kwd.
Assignee: jfrancis → glazman
nominating this as QAHP-top (from discussion with Sujay)
Whiteboard: [QAHP] EDITORBASE 5 days → [QAHP-top] EDITORBASE 5 days
Whiteboard: [QAHP-top] EDITORBASE 5 days → [QAHP-top] EDITORBASE+ 5 days
Keywords: nsbeta1+
fixing this will also involve rewriting every piece of editor code that looks at
lists.  This is a lot of stuff, including things which are seemingly unrelated
to lists.

It is a big deal and actually much more complex to code sublists the "right"
way, which is why we do them the "wrong" way.  Since every major browser
recognizes lists within lists, and does the right thing with them, I would
postpone this.
> fixing this will also involve rewriting every piece of editor code that looks
> at lists.

Not that I'd doubt your understanding of that code (you wrote most of it), but
why is that so?
- We don't "fix" existing, correct HTML that we load from a file to be wrong, do
we? (If we do, then this bug is more severe.)
- If not, then most code needs to deal with the correct version anyways. (If it
can't, then that's a much more severe bug.)
- If so, then it's just a matter of what happens when I do "increase indention"
etc. for a list item.

Where am I wrong?
Removing EDITORBASE+, nsbeta1+ for re-consideration
Keywords: nsbeta1+nsbeta1
Whiteboard: [QAHP-top] EDITORBASE+ 5 days → [QAHP-top] EDITORBASE 5 days
Ben : cut and paste
Bulk moving all nsbeta1 nominations to Moz1.1.  If the nomination is approved it
will be marked nsbeta1+ and moved to the Mozilla1.0 milestone.
Target Milestone: mozilla1.0 → mozilla1.1
User does not notice any difference, so is not EDITORBASE
Whiteboard: [QAHP-top] EDITORBASE 5 days → [QAHP-top] EDITORBASE- 5 days
Keywords: nsbeta1nsbeta1-
removing myself from the cc list
*** Bug 146774 has been marked as a duplicate of this bug. ***
Keywords: nsbeta1-nsbeta1
batch: adding topembed per Gecko2 document
Keywords: topembed
Topembed- as this is not currently blocking a major embedding customer.
Blocks: grouper
*** Bug 173212 has been marked as a duplicate of this bug. ***
seeking reconsideration for editorbase
If people are creating bad lists, we'll have to deal with bad lists some day
(when we won't want to).  The sooner we fix this, the less buggy files we'll
have to deal with later.
Whiteboard: [QAHP-top] EDITORBASE- 5 days → [QAHP-top] EDITORBASE 5 days
Target Milestone: mozilla1.1alpha → mozilla1.3alpha
EDITORBASE-. Meets the editorbase criteria, but we have other issues to go after
Whiteboard: [QAHP-top] EDITORBASE 5 days → [QAHP-top] EDITORBASE- 5 days
Whiteboard: [QAHP-top] EDITORBASE- 5 days → [QAHP-top] EDITORBASE- 5 days [t2]
Keywords: nsbeta1nsbeta1-
Replacement for nsHTMLEditRules::WillHTMLIndent() doing nesting of lists in
full conformance to HTML 4 spec, ie sublists have a list item container and not

another list like we do now.

Warning: this is code in progress, dirty, not optimized and the outdentation
counterpart is not covered by this patch.
Oh happy day!
Some things that will have to work before this could land:
1) outdent (as daniel indicates)
2) pasting of list structure into an existing list
3) changing the list type with a selection across multilevel lists
4) joining lists (including lists at different levels) via deletion
This list is off the top of my head and may not be exhaustive.

There is greater opportunity for users to get results they dont expect here. 
For instance, if there is something else besides the <ul> inside the parent
<li>, then there is a question abot what outdenting the <ul> means.  Keep it
inside the <li> but make it unlisted?  Or split the parent <li> and make the
sublist a bunch of <li>'s that are peers to original parent <li>?
Joe: oh, yes, that's far from done, I know
*** Bug 190929 has been marked as a duplicate of this bug. ***
*** Bug 212955 has been marked as a duplicate of this bug. ***
*** Bug 219265 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla1.3alpha → ---
*** Bug 262635 has been marked as a duplicate of this bug. ***
*** Bug 277967 has been marked as a duplicate of this bug. ***
This PHP code allow to fix the misteaks in nested lists 
$html = preg_replace("#</li>#","",$html);  
while( preg_match("#</ul>[^>]*<ul>[^>]*<ul>#",$html) )  
 $html = preg_replace("#</ul>[^>]*<ul>[^>]*<ul>#","<ul>",$html);  
$html = preg_replace("#</ul>#","</li></ul>",$html);  
the PHP code above make the same HTML as IE6. You have to add some code to 
close the li tags correctly. 
(In reply to comment #39)

> This PHP code allow to fix the misteaks in nested lists 

Please, this comment and the one after this one are useless here. We know how the
list is invalid and how the markup should be fixed. The problem is still very
complex because of copy/paste and indentation.
(In reply to comment #41)  
All right. I thought that this piece of code could maybe help someone. But I  
realize that it's not the right place to talk about this. 
QA Contact: sujay → editor
news here?
Assignee: daniel → ehsan
See Also: → 87617
Moving to p3 because no activity for at least 1 year(s).
See for more information
Priority: P2 → P3
Assignee: ehsan → nobody

Note that Chrome does the same thing on midasdemo:

  1. Type "abc"
  2. Insert unordered list
  3. Tap enter key and type "abc"
  4. Insert indent
  5. View HTML source to check <ul> directly includes another <ul>.

Is <ul> directly including another <ul> still invalid on modern HTML? It seems so but not sure.

Summary: Nested lists are incorrectly generated → execCommand("indent") unexpectedly directly nests list elements
Duplicate of this bug: 525888
Blocks: 1548753

Oops, mistake 🙄

Closed: 2 years ago
Resolution: --- → FIXED
See Also: → 485466, 1462572
Resolution: FIXED → ---
Duplicate of this bug: 1645330
Duplicate of this bug: 1647997
You need to log in before you can comment on or make changes to this bug.