54479 - execCommand("indent") unexpectedly directly nests list elements

Status: NEW

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

Description

[Dagfinn I. Mannsåker]

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

Comment 1

[Dagfinn I. Mannsåker]

Attachment: Correct and incorrect nesting example

Comment 2

[rubydoo123]

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

Comment 3

[rubydoo123]

neglected to set priority

Comment 4

[Joe Francis]

yea verily

Comment 5

[sujay]

this bug was FUTURE, but the keywords say "correctness" and "mozilla0.9"

changing Target milestone to mozilla0.9

Comment 6

[rubydoo123]

[e-mail] set the keywords without explanation

moving back to future, removing mozilla0.9

Comment 7

[Matthew Tuck [:CodeMachine]]

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.

Comment 8

[rubydoo123]

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.

Comment 9

[rubydoo123]

bringing into 1.0

Comment 10

[rubydoo123]

there is a side issue from bug 68132, where correctly nested lists are not
outdented correctly.

Comment 11

[Ben Bucksch (:BenB)]

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.

Comment 12

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Taking ownership of this bug in order to help jfrancis who has BR's selection
on his shoulders... Setting 5 days for EDITORBASE kwd.

Comment 13

[Kathleen :Brade]

nominating this as QAHP-top (from discussion with Sujay)

Comment 14

[Syd Logan]

plussing

Comment 15

[Joe Francis]

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.

Comment 16

[Ben Bucksch (:BenB)]

> 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?

Comment 17

[Kevin McCluskey (gone)]

Removing EDITORBASE+, nsbeta1+ for re-consideration

Comment 18

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Ben : cut and paste

Comment 19

[Kevin McCluskey (gone)]

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.

Comment 20

[Syd Logan]

User does not notice any difference, so is not EDITORBASE

Comment 21

[Kevin McCluskey (gone)]

nsbeta1-

Comment 22

[rubydoo123]

removing myself from the cc list

Comment 23

[Joe Francis]

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

Comment 24

[Marek Z. Jeziorek]

batch: adding topembed per Gecko2 document
http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html

Comment 25

[Jaime Rodriguez, Jr.]

Topembed- as this is not currently blocking a major embedding customer.

Comment 26

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

Comment 27

[Kathleen :Brade]

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.

Comment 28

[Kevin McCluskey (gone)]

EDITORBASE-. Meets the editorbase criteria, but we have other issues to go after
first.

Comment 29

[Kevin McCluskey (gone)]

nsbeta1-

Comment 30

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: work-in-progress #1

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.

Comment 31

[rubydoo123]

Oh happy day!

Comment 32

[Joe Francis]

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>?

Comment 33

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Joe: oh, yes, that's far from done, I know

Comment 34

[Allen]

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

Comment 35

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

Comment 36

[Elmar Ludwig]

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

Comment 37

[Boris Zbarsky [:bzbarsky]]

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

Comment 38

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

Comment 39

[Bieler Batiste]

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);  

Comment 40

[Bieler Batiste]

the PHP code above make the same HTML as IE6. You have to add some code to 
close the li tags correctly. 

Comment 41

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

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

Comment 42

[Bieler Batiste]

(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. 
 
Cheers 

Comment 43

[Giorgio]

news here?

Comment 44

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 45

[Kagami Rosylight [:saschanaz] (they/them)]

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.

Comment 47

[Kagami Rosylight [:saschanaz] (they/them)]

Oops, mistake 🙄

Comment 48

[Kagami Rosylight [:saschanaz] (they/them)]

Filed https://github.com/w3c/editing/issues/256 and https://github.com/whatwg/html/issues/5567.