[Webkit-unassigned] [Bug 52324] REGRESSION: A problem with Voiceover and finding links

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52324


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #79020|review?                     |review+
               Flag|                            |




--- Comment #8 from Darin Adler <darin at apple.com>  2011-01-18 10:33:09 PST ---
(From update of attachment 79020)
View in context: https://bugs.webkit.org/attachment.cgi?id=79020&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:1022
> +            IntRect frameRect = widget->frameRect();
> +            IntPoint newPoint = point;
> +            newPoint.setX(point.x() - frameRect.x());
> +            newPoint.setY(point.y() - frameRect.y());
> +            return axObjectCache()->getOrCreate(widget)->accessibilityHitTest(newPoint);

This can be written without doing X and Y separately:

    IntPoint newPoint = point - widget->frameRect().location();

Or even:

    return axObjectCache()->getOrCreate(widget)->accessibilityHitTest(point - widget->frameRect().location());

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1888
> +    // This should be the last check
> +    if (!helpText().isEmpty())
> +        return false;
>      
> -    return !m_renderer->isListMarker() && !isWebArea();
> +    return true;

If it is important that the helpText check be last then you *could* write it like this:

    return helpText().isEmpty();

Unless you think that’s less clear. Also, we prefer comments that are in sentence style.

Also, the comment needs to say why the helpText check should be last. The general rule of thumb is that comments say why and the code itself says what.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.