[webkit-reviews] review granted: [Bug 93998] [chromium] More Android changes to WebFrameImpl::selectRange : [Attachment 160944] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Iain Merrick <husky at chromium.org>'s
request for review:
Bug 93998: [chromium] More Android changes to WebFrameImpl::selectRange
https://bugs.webkit.org/show_bug.cgi?id=93998

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=160944&action=review


I'd really like rniwa to take a look at this patch when he gets back from
vacation.  Based on the discussion in the previous bug, this patch looks
plausible to me, but I'm far from an expert here.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:-1482
> -    if (frame()->selection()->shouldChangeSelection(newSelection))

Looks like we lost this call.  Is this no longer needed?

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1486
> +    if (selection.isNone())
> +	   return false;

Should this ASSERT?  Is it an error to call this function without a current
selection?