Bug 8587 – REGRESSION: {display:list-item} items outside an ol or ul element don't number correctly

• Component: CSS
• Status: RESOLVED FIXED

[leif halvard silli]

If one giv an HTML element the CSS display property div{display:list-item}, divs that follows one by another, should be numbered like in a regular OL-list. And that is how it works in the released version of Safari (version 2.0.3 — build 417.9.2). 

But in the nighly builds of the last mont (for instance the build of 25th of April), this no longer works.

This makes WEbKit an exception to Internet Explorer 5 (Mac OS X), Internet Explorer 6 (Windows), Opera 9beta and iCab. (Unfortunately, it doesn't work in Firefox either.)

So clearly WebKit is on the wrong track here. 

A  major severity bug a) because it is important to get generated content to work properly, b) it is especially important to make generated content work with the help of display:list-item, as this is also compatible with Internet Explorer - which otherwise does not support generated content. c) It breaks previous functionality of WebKit

[leif halvard silli]

Summary spelling correction

[Eric Seidel (no email)]

I fixed a crash recently in list items.  I likely caused this regression with my fix for bug 8542.

[leif halvard silli]

The issue is also present in the build from 24th of March. (I downloaded the new one to see if it was also there ..)

Thus your fix of bug 8542 dated 23rd of April, should not be the problem per se.

[Sam Weinig]

Confirming.  This is a regression from stock Safari (for 10.4.7).

[David Kilzer (:ddkilzer)]

Created attachment 9410 [details]
Test case from URL

[Darin Adler]

The change in behavior is due to the fix for bug 5776.

The issue isn't the use of display:list-item.

The issue is items that are outside an ol or ul element and how to group them into lists. I'll attach a patch that treats any groups of items outside ol or ul elements as lists, broken up by any ol or ul elements that appear, but I'm not sure if that's the correct behavior. Someone might expect the document nesting to affect the numbering instead.

[Darin Adler]

Created attachment 9446 [details]
patch with detailed change log and a layout test

[Alice Liu]

<rdar://problem/4631844>

[John Sullivan]

Comment on attachment 9446 [details]
patch with detailed change log and a layout test

r=me, assuming this matches the behavior of other browsers

[Darin Adler]

(In reply to comment #9)
> (From update of attachment 9446 [details] [edit])
> r=me, assuming this matches the behavior of other browsers

It doesn't match Firefox, but I believe it does match the others.

[Darin Adler]

Committed revision 15458.

[leif halvard silli]

(In reply to comment #11)
> Committed revision 15458.

Tested Nightly of 16th of July. The old behaviour has now _partly_ been restored. But the handeling of nested elements with display:list, is still different from what it was. If you ad two H2 elements with the same CSS display:list value, like this:

<DIV class=list>
   <H2 class=list>txt1</H2>
   <H2 class=list>txt2</H2>
</DIV>
<DIV class=list>....</DIV>

Then the relased Safari (+ the others) will number the first H2 as 1 and the secound H2 as 2 and the secound DIV as 2 again etc.  While the current nightly will number the first H2 as 2 and the secound H2 as 3. And the the secound DIV as 4. Etc. (IE6 will do the same - don't know about IE7.)  (You may change the H2 to nested DIV instad, if you wish.)

The old behaviour was the logic one.  It is illogical that an element within another  (In reply to comment #6:)

> Someone might expect the document  nesting to affect the numbering instead.

For sure. This is the old behaviour of Safari and the other browser. It is also the logical behaviour - unless one specify otherwise (using real CSS counters and not just display:list.) I changed my demo document to show what I mean.

[Darin Adler]

Comment on attachment 9446 [details]
patch with detailed change log and a layout test

Clearing the review flag on this.

[Darin Adler]

(In reply to comment #12)
> Tested Nightly of 16th of July. The old behaviour has now _partly_ been
> restored. But the handeling of nested elements with display:list, is still
> different from what it was. If you ad two H2 elements with the same CSS
> display:list value, like this:
> 
> <DIV class=list>
>    <H2 class=list>txt1</H2>
>    <H2 class=list>txt2</H2>
> </DIV>
> <DIV class=list>....</DIV>
> 
> Then the relased Safari (+ the others) will number the first H2 as 1 and the
> secound H2 as 2 and the secound DIV as 2 again etc.  While the current nightly
> will number the first H2 as 2 and the secound H2 as 3. And the the secound DIV
> as 4. Etc. (IE6 will do the same - don't know about IE7.)  (You may change the
> H2 to nested DIV instad, if you wish.)
> 
> The old behaviour was the logic one.  It is illogical that an element within
> another  (In reply to comment #6:)

OK. If you want me to fix this right, could you provide some test cases? For example, how should this be numbered?

    <div>
        <div class="listitem">a</div>
        <div>
            <div class="listitem">b</div>
        <div>
        <div class="listitem">c</div>
    </div>

And what about when tables are involved?

There's no way to "restore the old Safari behavior", but we can certainly fix this so it's 100% right if we have some test cases to demonstrate correct behavior.

[leif halvard silli]

Created attachment 9508 [details]
New test case with a nested list - works in IE, released Safari, iCab, Opera

[leif halvard silli]

(In reply to comment #14)

> OK. If you want me to fix this right, could you provide some test cases?

Se the new test case I attached.  (FWIW I have also edited my original page (on my original URL) so that it includes two nested DIVs with the listitem class inside the first listitem DIV.  )

>  For example, how should this be numbered?
> 
>     <div>
>         <div class="listitem">a</div>
>         <div>
>             <div class="listitem">b</div>
>         <div>
>         <div class="listitem">c</div>
>     </div>

This markup isn't valid. Was that your purpose? I am not Ian Hixie. I would not take upon myself the responsibilty for explaining error handeling ... ;-) If however you meant what I have demoed in my new test file - namely this:

<DIV>
<DIV><DIV class=LISTITEM>a</DIV></DIV>
<DIV><DIV class=LISTITEM>b</DIV></DIV>
<DIV><DIV class=LISTITEM>c</DIV></DIV>
</DIV>

then I would say that each .LISTITEM here should have number 1, because they are in fact 3 independent lists. (Of course, if you add .LISTITEMs one after another [like in my test case], they should be numbered #1, #2 - however.) In the new test case, the second level DIVs are considered a first level list, while the third level of DIVs (with the .listitem class) are second level (and in in lower-alpha) lists. 

> And what about when tables are involved?

Sounds tricky.  In my new test file, I inserted two TABLEs with the .listitem class. These tables are then counted just like the DIVs with the same class (in iCab, IE, Opera and released Safari). Is that what you meant?

[Ian 'Hixie' Hickson]

[e-mail] is the current expert on this, but he doesn't seem to have a bugzilla account here.

[Darin Adler]

(In reply to comment #16)
> (In reply to comment #14)
> 
> > OK. If you want me to fix this right, could you provide some test cases?
> 
> Se the new test case I attached.  (FWIW I have also edited my original page (on
> my original URL) so that it includes two nested DIVs with the listitem class
> inside the first listitem DIV.  )
> 
> >  For example, how should this be numbered?
> > 
> >     <div>
> >         <div class="listitem">a</div>
> >         <div>
> >             <div class="listitem">b</div>
> >         <div>
> >         <div class="listitem">c</div>
> >     </div>
> 
> This markup isn't valid. Was that your purpose?

Sorry, I meant:

    <div>
        <div class="listitem">a</div>
        <div>
            <div class="listitem">b</div>
        </div>
        <div class="listitem">c</div>
    </div>

If we get a nice set of tests cases, it should be easy to code up a solution.

[leif halvard silli]

Created attachment 9529 [details]
Test case 1, 2 and 3

The same test case I've allready made, with the addition of Darin's (now valid) test case - in two versions.

[leif halvard silli]

(In reply to comment #18)
> > This markup isn't valid. Was that your purpose?
> 
> Sorry, I meant:  [...]

Case 3 in my  new test case file, shows your exampe as you probably meant it (CSS wise). 

Hah, I see (and am reminded about) that IE displays Case 3 in this file differently from released Safari, Opera and iCab [so what I write in the test file is wrong]. In IE the last .listitem gets counted as number 3, while it in the the other browsers gets counted  as number 2. Which means that IE even counts the DIV-element without the .LISTITEM class. 

And, hence, if you do this,

<DIV><P>A<P>B<DIV class=listitem>B</DIV></DIV>

then IE6 will count the .listitem here as number 3 — because of the preceding two block elements (the two P-elements). Which must be wrong, even if it is possible to understand how it thinks ... 

At the same time, I guess this was in fact what your question was about. I think I have answered as much as I am able to ... 

[Darin Adler]

So I think the algorithm here is that if there is no list element, any peer DOM nodes each form a separate list. If nodes don't have the same parent, then they are part of a separate list.

And in the case where there *is* a list element, all DOM nodes inside the list are peers even if they are at different nesting levels.

I'm not sure that's right, but that's what it looks like to me given the test cases so far.

[Andrew Wellington]

Created attachment 9626 [details]
Proposed patch

This patch should fix the problem using the algorithm suggested by darin.

[Darin Adler]

Comment on attachment 9626 [details]
Proposed patch

This is great as-is, so r=me.

Should have a period at the end of the comment to match the other comments in the function.

It's a tiny bit inelegant to do traverseNextNode() inside that "if (otherList)" case, because we end up doing traversePreviousNode() right after that, which reverses it. However, I don't see a way to restructure the loop to avoid this without making it more complicated and harder to read. The comment doesn't make it entirely clear that the traverseNextNode is right simply because of the traversePreviousNode coming up.

If I was writing the enclosingList function I'd make a local variable called parent, since node->parentNode() is used twice. But that might just be a habit of mine -- not really sure it's good programming style.

[David Kilzer (:ddkilzer)]

Committed second patch as r15687.  See also Comment #11.

[Darin Adler]

Committed revision 15687.