[Webkit-unassigned] [Bug 36724] Add support for <ol reversed>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36724





--- Comment #15 from Jakub Wieczorek <jwieczorek at webkit.org>  2010-05-08 03:30:36 PST ---
(In reply to comment #14)
> (In reply to comment #13)
> > "The default value, used if the attribute is missing or if the value cannot be
> > converted to a number according to the referenced algorithm, is 1 if the
> > element has no reversed attribute, and is the number of child li elements
> > otherwise."
> 
> This start attribute is an example, then, of something in the DOM API that
> requires that the render tree be up to date to give a correct response. Thus I
> think we'll need a call to updateLayout for the same reason that functions like
> Element::offsetHeight do.
> 
> Except that it specifically says the "number of child li elements", which
> ignores the CSS "display: list-item".
> 
> If the text is really intentional about stating the number of child li
> elements, then we should remove all the code and smarts that use the render
> tree and create test cases that demonstrate the incorrect looking numbering you
> get when there are "display: list-item" items that are not li elements.

Oh, I overlooked it talking about li elements rather than just list items.

If the text is really intentional, then I think it's just wrong. At least it
seems to be in conflict with CSS in this matter. I am not convinced we should
follow this particular recommendation.

> > > It's also not great that the item count is stored in the DOM tree when it's
> > > dependent on the rendering. The count should be stored in the render tree even
> > > if the DOM tree might access it. I think this belongs in the renderer class for
> > > the list not the DOM class.
> > 
> > Yeah, I agree with that. I'm going to refactor it into a RenderList class that
> > will serve as a renderer for lists.
> 
> Now I’m a little worried about my suggestion. If there is not already a
> RenderList class, then allocating a renderer of that class instead of the
> renderer type that we are already creating might be a problem for lists that
> have CSS "display: inline" or something like that applied to them.

Yes, I think you're right.

>From what I can see, that would mean we can't really move this logic to the
rendering tree. A list on the rendering side can be virtually anything whereas
on the DOM side we exactly know that the only place where we want to keep track
of how many list items a list contains is HTMLOListElement - this might change
in the future as the need arises but I think that is true now.

Therefore I suggest we get back to the previous approach. The second major
change you suggested - putting the renumbering logic in one place should be
still doable with a static function in RenderListItem (I agree, not a perfect
place for this).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.