[webkit-reviews] review granted: [Bug 50737] Implement "Use Selection for Find" in WebKit2 : [Attachment 76024] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Maciej Stachowiak <mjs at apple.com>'s request for
review:
Bug 50737: Implement "Use Selection for Find" in WebKit2
https://bugs.webkit.org/show_bug.cgi?id=50737

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

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=76024&action=review

r=me if you change enabledCopy to enabledTake…

> WebCore/editing/EditorCommand.cpp:1510
> +	   { "TakeFindStringFromSelection", {
executeTakeFindStringFromSelection, supportedFromMenuOrKeyBinding, enabledCopy,
stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },

I think you meant to use enabledTakeFindFromSelection here, not enabledCopy.

> WebCore/editing/mac/EditorMac.mm:200
> +	   systemBeep();

Not sure it’s better to call the cross-platform function here, since this is
Mac code, and you can just NSBeep() without incurring the #import cost.

> WebCore/editing/mac/EditorMac.mm:204
> +    NSString* nsSelectedText =
m_frame->displayStringModifiedByEncoding(selectedText());

Misplaced star.