Bug 136247 - AX: Heuristic: Avoid exposing an element as clickable if mouse event delegation is handled on an AXElement with more than one descendant AXElement
Summary: AX: Heuristic: Avoid exposing an element as clickable if mouse event delegati...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on: 133613
Blocks:
  Show dependency treegraph
 
Reported: 2014-08-26 01:20 PDT by James Craig
Modified: 2015-04-04 23:52 PDT (History)
10 users (show)

See Also:


Attachments
patch (14.25 KB, patch)
2015-03-31 17:38 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (14.25 KB, patch)
2015-03-31 18:03 PDT, chris fleizach
no flags Details | Formatted Diff | Diff
patch (14.01 KB, patch)
2015-03-31 23:31 PDT, chris fleizach
mario: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Craig 2014-08-26 01:20:24 PDT
Add-on from Bug 133613.

Chris also added: "We could not expose 'clickable' on an object if there is a more than say two objects with the same event handler."

There will be cases matching that heuristic that we can't disambiguate, but I think this will result in better behavior overall, so let's try it. I wrote it up as "more than one descendant AXElement" as opposed to "DOM Element" b/c if the other DOM elements aren't turned into accessibles, they won't matter.…
Comment 1 Radar WebKit Bug Importer 2014-08-26 01:20:48 PDT
<rdar://problem/18130664>
Comment 2 chris fleizach 2015-03-31 17:38:06 PDT
Created attachment 249871 [details]
patch
Comment 3 James Craig 2015-03-31 17:56:28 PDT
Comment on attachment 249871 [details]
patch

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

> Source/WebCore/accessibility/AccessibilityObject.cpp:2019
> +    // [Bug: 136247] Heuristic: element handlers that have more than one accessible descendant should not be exposed as supporting press.
> +    if (actionElement != element()) {
> +        if (AccessibilityObject* axObj = axObjectCache()->getOrCreate(actionElement)) {
> +            AccessibilityChildrenVector results;
> +            // Search within for immediate descendants that are static text. If we find more than one
> +            // then this is an event delegator actionElement and we should expose the press action.
> +            Vector<AccessibilitySearchKey> keys({ StaticTextSearchKey, ControlSearchKey, GraphicSearchKey, HeadingSearchKey, LinkSearchKey });
> +            AccessibilitySearchCriteria criteria(axObj, SearchDirectionNext, "", 2, false, false);
> +            criteria.searchKeys = keys;
> +            axObj->findMatchingObjects(&criteria, results);
> +            if (results.size() > 1)
> +                return false;
> +        }
> +    }
> +    

If I'm reading this correctly, this might mean a link element inside an event delegate would not accept press?

<div onclick="foo()">
  <div role="link" tabindex="0">regardless of delegation, this node should still support "press" due to the role</div>
</div>
Comment 4 chris fleizach 2015-03-31 18:00:13 PDT
Links are always going to support press

(In reply to comment #3)
> Comment on attachment 249871 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249871&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:2019
> > +    // [Bug: 136247] Heuristic: element handlers that have more than one accessible descendant should not be exposed as supporting press.
> > +    if (actionElement != element()) {
> > +        if (AccessibilityObject* axObj = axObjectCache()->getOrCreate(actionElement)) {
> > +            AccessibilityChildrenVector results;
> > +            // Search within for immediate descendants that are static text. If we find more than one
> > +            // then this is an event delegator actionElement and we should expose the press action.
> > +            Vector<AccessibilitySearchKey> keys({ StaticTextSearchKey, ControlSearchKey, GraphicSearchKey, HeadingSearchKey, LinkSearchKey });
> > +            AccessibilitySearchCriteria criteria(axObj, SearchDirectionNext, "", 2, false, false);
> > +            criteria.searchKeys = keys;
> > +            axObj->findMatchingObjects(&criteria, results);
> > +            if (results.size() > 1)
> > +                return false;
> > +        }
> > +    }
> > +    
> 
> If I'm reading this correctly, this might mean a link element inside an
> event delegate would not accept press?
> 
> <div onclick="foo()">
>   <div role="link" tabindex="0">regardless of delegation, this node should
> still support "press" due to the role</div>
> </div>
Comment 5 chris fleizach 2015-03-31 18:03:39 PDT
Created attachment 249874 [details]
patch
Comment 6 chris fleizach 2015-03-31 23:31:25 PDT
Created attachment 249896 [details]
patch
Comment 7 Mario Sanchez Prada 2015-04-04 15:17:37 PDT
Comment on attachment 249896 [details]
patch

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

> Source/WebCore/ChangeLog:8
> +        Modify the logic for determing whether an element supports the press action by trying to filter out objects being handled by event delegation.

typo: s/determing/determining
Comment 8 chris fleizach 2015-04-04 23:52:38 PDT
http://trac.webkit.org/changeset/182355