[webkit-reviews] review granted: [Bug 60403] [HTML5] Implement the selectionDirection property on input and textarea : [Attachment 102441] Reverted Objective-C binding change

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 60403: [HTML5] Implement the selectionDirection property on input and
textarea
https://bugs.webkit.org/show_bug.cgi?id=60403

Attachment 102441: Reverted Objective-C binding change
https://bugs.webkit.org/attachment.cgi?id=102441&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=102441&action=review


> Source/WebCore/html/HTMLTextFormControlElement.cpp:219
> +    DEFINE_STATIC_LOCAL(String, forward, ("forward"));
> +    DEFINE_STATIC_LOCAL(String, backward, ("backward"));
> +
> +    TextFieldSelectionDirection direction = SelectionHasNoDirection;
> +    if (directionString == forward)
> +	   direction = SelectionHasForwardDirection;
> +    else if (directionString == backward)
> +	   direction = SelectionHasBackwardDirection;

It occurs to me that comparing a String with another String is not
significantly faster than comparing a String with a const char*, so we might
not get any benefit from\ those static local strings at all.

> Source/WebCore/html/HTMLTextFormControlElement.cpp:347
> +TextFieldSelectionDirection
HTMLTextFormControlElement::computeSelectionDirection() const
> +{
> +    Frame* frame = document()->frame();
> +    if (!frame)
> +	   return SelectionHasNoDirection;

Should this function ASSERT(isTextFormControl())?