Bug 146300 - AX: AccessibilityObject focus events that don't cause a selection change can leave m_isSynchronizingSelection set to true
Summary: AX: AccessibilityObject focus events that don't cause a selection change can ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-06-24 17:06 PDT by Doug Russell
Modified: 2015-06-25 18:01 PDT (History)
10 users (show)

See Also:


Attachments
patch (10.69 KB, patch)
2015-06-24 17:11 PDT, Doug Russell
no flags Details | Formatted Diff | Diff
patch (11.62 KB, patch)
2015-06-25 17:00 PDT, Doug Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Doug Russell 2015-06-24 17:06:15 PDT
If an accessibilityObject is focused, it calls cache->setIsSynchronizingSelection(true) assuming that the focus will cause a selection change, but doesn't validate that the change occurred or reset this state. After any event that's assuming a selection change will happen, m_isSynchronizingSelection should be reset to false.
Comment 1 Radar WebKit Bug Importer 2015-06-24 17:10:07 PDT
<rdar://problem/21536941>
Comment 2 Doug Russell 2015-06-24 17:11:45 PDT
Created attachment 255531 [details]
patch
Comment 3 chris fleizach 2015-06-24 22:51:20 PDT
Comment on attachment 255531 [details]
patch

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

> LayoutTests/ChangeLog:3
> +        Bug 146300 AX: AccessibilityObject focus event that doesn't cause a selection 

"focus events that don't cause a"

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1513
> +    AXObjectCache* cache = renderObject.axObjectCache();

It looks like this method might be better off taking a point to the axobject cache

> Source/WebCore/accessibility/mac/AXObjectCacheMac.mm:337
> +        [rootWebArea()->wrapper() accessibilityPostedNotification:NSAccessibilityFocusedUIElementChangedNotification userInfo:nil];

since this looks like it's only for testing, i would change the name to something like "AXFocusedChanged" so that when checking this in the test it will be clear what the value should be, and
will also let other platforms follow suit
Comment 4 Doug Russell 2015-06-25 17:00:32 PDT
Created attachment 255597 [details]
patch
Comment 5 Doug Russell 2015-06-25 17:01:32 PDT
I changed to AXFocusedChanged, but it's worth noting that other logic can still post NSAccessibilityFocusedUIElementChangedNotification so we may end up writing tests some times that need to listen for both. Probably not an issue, but thought I'd note it.
Comment 6 WebKit Commit Bot 2015-06-25 18:01:12 PDT
Comment on attachment 255597 [details]
patch

Clearing flags on attachment: 255597

Committed r185974: <http://trac.webkit.org/changeset/185974>
Comment 7 WebKit Commit Bot 2015-06-25 18:01:17 PDT
All reviewed patches have been landed.  Closing bug.