Bug 196988 – [ContentChangeObserver] Use aria role as a hint whether a tap should result in a synthetic click

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

role="checkbox" e.g.

[Radar WebKit Bug Importer]

<rdar://problem/49955328>

[alan]

Created attachment 367581 [details]
Patch

[chris fleizach]

Comment on attachment 367581 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:569
> +static bool isAriaRoleForImmediateClick(AccessibilityRole role)

feels like this might better live in AccessibilityObject. There's an existing

bool AccessibilityObject::isARIAControl(AccessibilityRole ariaRole) that might be appropriate

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:572
> +        || role == AccessibilityRole::Link || role == AccessibilityRole::List || role == AccessibilityRole::ListBox || role == AccessibilityRole::ListBoxOption || role == AccessibilityRole::ListItem 

not sure we want to tap ListBox

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:575
> +        || role == AccessibilityRole::SearchField || role == AccessibilityRole::Slider || role == AccessibilityRole::SliderThumb || role ==AccessibilityRole::SpinButton || role == AccessibilityRole::SpinButtonPart

spacing 
role ==AccessibilityRole::SpinButton

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:576
> +        || role == AccessibilityRole::TextArea || role == AccessibilityRole::TextField || role == AccessibilityRole::ToggleButton;

do we want to tap text areas and text fields?

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:577
> +}

this might be better written as a switch statement

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:590
> +    return isAriaRoleForImmediateClick(AccessibilityObject::ariaRoleToWebCoreRole(ariaRoleString));

ariaRoleToWebCoreRole should handle null and empty strings. you can probably remove this if

[alan]

Created attachment 367641 [details]
Patch

[alan]

(In reply to chris fleizach from comment #3)
> Comment on attachment 367581 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367581&action=review
> 
> > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:569
> > +static bool isAriaRoleForImmediateClick(AccessibilityRole role)
> 
> feels like this might better live in AccessibilityObject. There's an existing
> 
> bool AccessibilityObject::isARIAControl(AccessibilityRole ariaRole) that
> might be appropriate
I didn't know about isARIAControl and isARIAInput! Thanks!

> > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:590
> > +    return isAriaRoleForImmediateClick(AccessibilityObject::ariaRoleToWebCoreRole(ariaRoleString));
> 
> ariaRoleToWebCoreRole should handle null and empty strings. you can probably
> remove this if
Good idea.

[WebKit Commit Bot]

Comment on attachment 367641 [details]
Patch

Clearing flags on attachment: 367641

Committed r244392: <https://trac.webkit.org/changeset/244392>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.