[Webkit-unassigned] [Bug 33668] WebCore::InsertListCommand::modifyRange infinite loop (100% CPU usage)

[bugzilla-daemon at webkit.org]

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


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62374|0                           |1
        is obsolete|                            |
  Attachment #62487|                            |review?
               Flag|                            |




--- Comment #14 from Ryosuke Niwa <rniwa at webkit.org>  2010-07-23 22:58:53 PST ---
Created an attachment (id=62487)
 --> (https://bugs.webkit.org/attachment.cgi?id=62487)
fixed per ojan's comments

(In reply to comment #13)
> This doesn't match what I get in IE8. I get:
> <P>One</P> 
> <P>Two</P> 
> <UL> 
> <LI>Three</LI> 
> <LI>Four</LI></UL>

I think you did InsertOrderedList as supposed to InsertUnorderedList.  When I do InsertUnorderedList on MSIE, I get:
<ul>
<li>one</li>
<li>two</li>
<ul>
<li>three</li>
<li>four</li>
</ul>
</ul>

But this is visually identical to
<ul>
<li>one</li>
<li>two</li>
</ul>
<ol>
<ul>
<li>three</li>
<li>four</li>
</ul>
</ol>

> > +++ LayoutTests/editing/execCommand/5207369-expected.txt	(working copy)
> > -<ul><li>One</li><li>Two</li><li>Three</li><li>Four</li></ul>
> > +<ul><li>One</li><li>Two</li></ul><ol><ul><li>Three</li><li>Four</li></ul></ol>
> 
> This output doesn't look right to me. Why is the ul wrapped in an ol? FWIW, our current behavior matches Opera. Firefox just inserts and LI at the top (clearly wrong). IE removes the outer list entirely. In either case, this new output doesn't match other browsers and seems worse than the old output.

Again, I think you did InsertOrderedList.

> Seems like the ideal behavior, from a user perspective, would be to output something like <ul><li>One</li><li>Two</li><ul><li>Three</li><li>Four</li></ul></ul>.

Right.  That's what I'm trying to accomplish here but with our current implementation, we can't tell if the outer list is contained within the selection because we modify the selection.  So the best we can do is to leave the outer OL intact.  I could add a logic to merge outer lists when the only content of the list is another list.  That will produce the above HTML as desired.  Ideally, when the entire list is selected, we can convert the type of the list element without having to work on each paragraph separately and then merge.

> > +r.setStart(e, 0);
> > +r.setEnd(e, e.childNodes.length);
> > +s.removeAllRanges();
> > +s.addRange(r);
> These four lines can just be s.selectAllChildren(e);

Good point.

> > +document.execCommand('InsertOrderedList', false, null);
> > +
> > +Markup.description("This tests crash when listifying nested lists with an orphaned list child in between (see bug 33668).")
> Technically, it's an infinite loop, not a crash, right?

Right.

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