[webkit-reviews] review denied: [Bug 75941] [crash] Renderer crashes when spell checking a disabled input field. : [Attachment 121832] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Shinya Kawanaka
<shinyak at chromium.org>'s request for review:
Bug 75941: [crash] Renderer crashes when spell checking a disabled input field.
https://bugs.webkit.org/show_bug.cgi?id=75941

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

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


> Source/WebKit/chromium/src/WebFrameImpl.cpp:1127
> +    if (!frame()->selection()->isContentEditable())
> +	   return;
> +

This check should be done inside Editor::replaceSelectionWithText

> Tools/DumpRenderTree/chromium/LayoutTestController.cpp:809
> +void LayoutTestController::replaceSelection(const CppArgumentList&
arguments, CppVariant* result)
> +{
> +    if (arguments.size() <= 0 || !arguments[0].isString()) {
> +	   result->setNull();
> +	   return;
> +    }
> +
> +    std::string text = arguments[0].toString();
> +   
m_shell->webView()->focusedFrame()->replaceSelection(WebString::fromUTF8(text))
;
> +}

I'm not certain it's such a good idea to add a new method on
layoutTestController for this one specific test. It's probably okay not to add
a test for this fix or add a manual test.