[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 #63036|0                           |1
        is obsolete|                            |
  Attachment #63104|                            |review?
               Flag|                            |




--- Comment #21 from Ryosuke Niwa <rniwa at webkit.org>  2010-07-30 14:54:43 PST ---
Created an attachment (id=63104)
 --> (https://bugs.webkit.org/attachment.cgi?id=63104)
fixed various problems.

(In reply to comment #20)
> (From update of attachment 63036 [details])
> > +            ExceptionCode ec = 0;
> > +            if (rangeStartIsInList && newList)
> > +                currentSelection->setStart(newList->parentNode(), newList->nodeIndex(), ec);
> > +            if (rangeEndIsInList && newList)
> > +                currentSelection->setEnd(newList->parentNode(), newList->nodeIndex()+1, ec);
> 
> There should be spaces around the "+" operator here where we do "+ 1".

Modified the approach.  Used the positions inside the list instead.

> If you're going to ignore the exception code, there is no need to initialize it to 0.

Fixed.

> Are all the code changes here covered by this single test?

When I modified the test to make sure rangeEndIsInList, there were other problems with my code
to convert the entire list.  So I fixed them all and also this exposed a bug in doApply where endOfSelection / startOfLastParagraph
can be orphaned / null.  Decided to save indexForVisiblePosition and restore it later.
I know it's a dirty trick but because moveParagraphWithClones removes the existing nodes, there is no other way to restore them.

I'm really worn away by all sorts of bugs in editing. For example, TextIterator has a bug of not counting empty LI,
which causes index to shit by 1.  And that's the whole reason, I have that ugly if statement to execute that line only if necessary.
But this bug in TextIterator will eventually expose if we added sufficiently more test cases.

We need a better of keeping track of positions on nodes that are moved / removed.  InsertListCommand / IndentOutdentCommand
are very fragile preciously because moveParagraph / moveParagraphWithClones remove some nodes that these two
commands are using to iterate through paragraphs.

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