[webkit-reviews] review granted: [Bug 8587] REGRESSION: {display:list-item} items outside an ol or ul element don't number correctly : [Attachment 9626] Proposed patch

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 8587: REGRESSION: {display:list-item} items outside an ol or ul element
don't number correctly
http://bugzilla.opendarwin.org/show_bug.cgi?id=8587

Attachment 9626: Proposed patch
http://bugzilla.opendarwin.org/attachment.cgi?id=9626&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
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.