[webkit-reviews] review denied: [Bug 75171] NULL ptr in WebCore::TextIterator::rangeFromLocationAndLength : [Attachment 121168] proposed patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Pablo Flouret
<pablof at motorola.com>'s request for review:
Bug 75171: NULL ptr in WebCore::TextIterator::rangeFromLocationAndLength
https://bugs.webkit.org/show_bug.cgi?id=75171

Attachment 121168: proposed patch
https://bugs.webkit.org/attachment.cgi?id=121168&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=121168&action=review


>>>>> Source/WebCore/editing/ApplyBlockElementCommand.cpp:84
>>>>> +    if (startOfSelection.isNull() || endOfSelection.isNull())
>>>> 
>>>> You should just call isNonOrphanedCaretOrRange on selection.
>>> 
>>> That's called at the start of the function and it returns true. If i
understand things correctly, it's not orphaned, it's just that there's a single
element but it's not selectable due to the -webkit-user-select css declaration.

>>> 
>>> A similar bug is https://bugs.webkit.org/show_bug.cgi?id=26214
>> 
>> selectionForParagraphIteration is probably returning a null selection.
isNonOrphanedCaretOrRange checks nullness as well as orphanedness.
> 
> selectionForParagraphIteration seems to deal with tables, and there are none
here, i checked the value of selection.isNonOrphanedCaretOrRange() in the
debugger anyway and it's true at that point also.
> 
> The selection is not null itself, selection.visible{Start,End} are the ones
returning null positions due to no available selectable elements (from my guess
given above).

That seems to indicate we have a bug. visibleStart/visibleEnd should null iff
selection is null. r- because this isn't the right fix.