[Webkit-unassigned] [Bug 179068] AX: search predicate returns containing group for plain text instead of text element

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com
 Attachment #326217|review?                     |review+
              Flags|                            |

--- Comment #15 from Darin Adler <darin at apple.com> ---
Comment on attachment 326217
  --> https://bugs.webkit.org/attachment.cgi?id=326217
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326217&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:806
> +    if (ariaRoleAttribute() == AccessibilityRole::StaticText)
> +        return true;
> +
> +    if (is<RenderText>(*m_renderer))
> +        return true;
> +
> +    if (isTextControl())
> +        return true;

It might make the line a little long, but I think a horizontal version of this function that uses || might be clearer than a string of if statements where it’s hard to spot which are "return true" and which are "return false". Something roughly like this:

    return isARIAStaticText() || is<RenderText>(m_renderer) || isTextControl();

Needs a little rearranging and better helper functions, but something to think about when writing future code.

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3567
>      if (!m_renderer)
>          return false;
> -    
> +
> +    if (!canHavePlainText())
> +        return false;

No need to check m_renderer for null in both hasPlainText and canHavePlainText.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171109/869bae1f/attachment.html>