[webkit-reviews] review denied: [Bug 37950] Crash in WebCore::TextIterator::handleTextNode() encountered in Google rich-text products : [Attachment 54133] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has denied Tony Chang (Google) <tony at chromium.org>'s request
for review:
Bug 37950: Crash in WebCore::TextIterator::handleTextNode() encountered in
Google rich-text products
https://bugs.webkit.org/show_bug.cgi?id=37950

Attachment 54133: Patch
https://bugs.webkit.org/attachment.cgi?id=54133&action=review

------- Additional Comments from mitz at webkit.org
Thanks for tackling this bug!

> -    if (!renderer->firstTextBox() && str.length() > 0) {
> -	   m_lastTextNodeEndedWithCollapsedSpace = true; // entire block is
collapsed space
> +    if (!renderer->firstTextBox()) {
> +	   if (str.length() > 0)
> +	       m_lastTextNodeEndedWithCollapsedSpace = true; // entire block is
collapsed space
>	   return true;
>      }

I am afraid that this fix may be wrong, because it doesn’t reset m_textBox in
this case. I would feel much better with a fix that didn’t return early here,
but instead just changed this

m_textBox = renderer->containsReversedText() ? m_sortedTextBoxes[0] :
renderer->firstTextBox();

to say

m_textBox = renderer->containsReversedText() ? (m_sortedTextBoxes.size() ?
m_sortedTextBoxes[0] : 0) : renderer->firstTextBox();