[Webkit-unassigned] [Bug 37060] List item markers are not always updated after changes in the DOM

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Darin Adler <darin at apple.com>  2010-04-05 07:57:03 PST ---
(In reply to comment #5)
> Created an attachment (id=52526)
 --> (https://bugs.webkit.org/attachment.cgi?id=52526) [details]
> patch
> 
> (In reply to comment #4)
> > (From update of attachment 52512 [details] [details])
> > >  static Node* enclosingList(const RenderObject* renderer)
> > >  {
> > > -    Node* node = renderer->node();
> > > -    if (node)
> > > -        return enclosingList(node);
> > > -
> > > -    renderer = renderer->parent();
> > >      while (renderer && !renderer->node())
> > >          renderer = renderer->parent();
> > >  
> > > -    node = renderer->node();
> > > +    if (!renderer)
> > > +        return 0;
> > > +
> > > +    Node* node = renderer->node();
> > >      if (isList(node))
> > >          return node;
> > 
> > This change means the code now walks up the render tree before walking up the
> > DOM tree. How did you determine that there was no case where that changed the
> > function result?
> 
> Actually it was doing this before too

Huh? The old code says if there is a node, then call enclosingList, walking up
the DOM tree, and don't go up the render tree at all. The new code does not do
that. We can debate that later if needed when we try to land this change, I
guess.

> Anyway, I'll just revert
> this hunk as it's not even necessary for this patch.

All we need to land this is a test case demonstrating what the code change
fixes.

> I changed it to be a for loop but personally I don't find it easier to read as
> we're ending up with something like:
> 
> if (child->node() && isList(child->node())) {
>     // We've found a nested, independent list: nothing to do here.
>     child = child->nextInPreOrderAfterChildren(list)->previousInPreOrder();
>     continue;
> }

Yes, that's not good. You should not let it be like that! Lets go back to the
way you have it.

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