Bug 146300

Summary: AX: AccessibilityObject focus events that don't cause a selection change can leave m_isSynchronizingSelection set to true
Product: WebKit Reporter: Doug Russell <d_russell>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
patch none

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.