[webkit-reviews] review granted: [Bug 26900] AX: Manual spell check with Command-; does not bring up suggestions : [Attachment 32142] patch

[bugzilla-daemon at webkit.org]

Beth Dakin <bdakin at apple.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 26900: AX: Manual spell check with Command-; does not bring up suggestions
https://bugs.webkit.org/show_bug.cgi?id=26900

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

------- Additional Comments from Beth Dakin <bdakin at apple.com>
> +IntPoint AccessibilityRenderObject::clickPoint() const
> +{
> +    // if we're asked to perform a show menu on an editable web area,
> +    // the click point should be where the selection is
> +    if (isWebArea() && !isReadOnly()) {
> +	   VisibleSelection visSelection = selection();
> +	   VisiblePositionRange range =
VisiblePositionRange(visSelection.visibleStart(), visSelection.visibleEnd());
> +	   IntRect bounds = boundsForVisiblePositionRange(range);
> +#if PLATFORM(MAC)
> +	  
bounds.setLocation(m_renderer->document()->view()->screenToContents(bounds.loca
tion()));
> +#endif	 
> +	   return IntPoint(bounds.x() + (bounds.width() / 2), bounds.y() -
(bounds.height() / 2));
> +    }
> +
> +    return AccessibilityObject::clickPoint();
> +}

This is just a style nit-pick, but how about you do the generic case as an
early return like:

if (!isWebArea() || isReadOnly())
    return AccessibilityObject::clickPoint();

VisibleSelection visSelection = selection();
…




r=me!