[Webkit-unassigned] [Bug 139383] Hang in TextCheckingHelper when checking a document with no characters

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Dec 7 20:39:01 PST 2014


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

--- Comment #1 from Daniel Jalkut <jalkut at red-sweater.com> ---
The nut of the buggy behavior at least as it pertains to the example HTML attached, is:

1. totalRangeLength is computed to be 1 (I'm fuzzy on why but I think it's because TextIterator's guts end up synthesizing a newline character for some reason).

2. while (totalLengthProcessed < totalRangeLength) opens an infinite loop that can only be escaped by a break statement or by the totalRangeLength (1) being exceeded by totalLengthProcessed.

3. totalLengthProcessed stays pinned at 0 because "currentLength = TextIterator::rangeLength(paragraphRange.get())" always returns 0, owing to paragraphRange having been MODIFIED after the initial totalRangeLength was recorded. The modifications are in the block of code with comment starting "Expand the search range", and comprising:

    RefPtr<Range> paragraphRange = m_range->cloneRange(IGNORE_EXCEPTION);
    setStart(paragraphRange.get(), startOfParagraph(m_range->startPosition()));
    int totalRangeLength = TextIterator::rangeLength(paragraphRange.get());
    setEnd(paragraphRange.get(), endOfParagraph(m_range->startPosition()));

Note that totalRangeLength is recorded before the setEnd call to paragraphRange, but the bulk of this method call will involve attempting to exhaust the content of paragraphRange.

Therefore my naive proposal is that the bug can be avoided, and more correct behavior in general be obtained, by moving the "int totalRangeLength =" line to AFTER the setEnd call, so that it is established before the loop starts that the totalRangeLength for practical purposes is 0. This causes the meat of the loop to never be entered, and at least for my test case, no hang any longer occurs.

At least two potential downsides to my simple solution come to mind:

1. totalRangeLength currently is obtained BEFORE the setEnd call for some good reason. I can't fathom what it might be, but hopefully some discussion of this bug could draw in a developer with more experience in the editing component.

2. Possibly the the fact that totalRangeLength returns 1 even when it does reflects a flaw in the TextIterator::rangeLength() method. If so, I can imagine that somebody more experienced with how that class should work would rightly argue that instead of fiddling with the order of things in TextCheckingHelper, the TextIterator should consistently return 0 (i.e. not synthesize a newline?) for any range that has no meaningful text content.

I'm hoping to get some good feedback from experienced folks, after which I'd be happy to follow up with a proper patch, assuming it's in the realm of something I'm competent  to do ;)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141208/6bd2869c/attachment-0002.html>


More information about the webkit-unassigned mailing list