AX: Fix disabled-controls-not-focusable.html in isolated tree mode by updating AXPropertyName::CanSetFocusAttribute when the disabled attribute changes.
<rdar://problem/87613404>
Created attachment 449207 [details] Patch
(In reply to Tyler Wilcock from comment #2) > Created attachment 449207 [details] > Patch --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ a/Source/WebCore/accessibility/AXObjectCache.cpp + case AXDisabledStateChanged: + tree->updateNodeProperty(*notification.first, AXPropertyName::CanSetFocusAttribute); + break; Add this same case to the updateIsolatedTree that takes a single notification as opposed to a vector of notifications. --- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h +++ a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h @@ -139,6 +139,7 @@ public: JSRetainPtr<JSStringRef> stringAttributeValue(NSString *attribute) const; double numberAttributeValue(NSString *attribute) const; RetainPtr<id> attributeValue(NSString *) const; + bool isAttributeSettable(NSString *) const; Can we make this private? --- a/LayoutTests/accessibility/disabled-controls-not-focusable.html +++ a/LayoutTests/accessibility/disabled-controls-not-focusable.html + async function checkControl(id) { + window.element = document.getElementById(id); + debug(`Testing #${id}`); + element.focus(); + await waitFor(() => { + window.axElement = accessibilityController.focusedElement; + return axElement && accessibilityController.accessibleElementById(id); + }); + shouldBeTrue("axElement.isFocusable"); + debug(`Setting the disabled attribute of element #${id}`); + element.disabled = true; + await expectAsyncExpression("axElement.isFocusable", "false"); + debug(""); + } The order of operations looks weird to me. I think it should be: 1. Check that an axElement is focusable. 2. Then focus it. 3. Disable the DOM element. 4. Check that now is not focusable. Not sure that we even have to do 2., because that's not what the test is testing, correct?
Created attachment 449221 [details] Patch
Thanks for the review -- addressed all of your comments in the latest patch.
(In reply to Andres Gonzalez from comment #3) > (In reply to Tyler Wilcock from comment #2) > > Created attachment 449207 [details] > > Patch > > --- a/Source/WebCore/accessibility/AXObjectCache.cpp > +++ a/Source/WebCore/accessibility/AXObjectCache.cpp > > + case AXDisabledStateChanged: > + tree->updateNodeProperty(*notification.first, > AXPropertyName::CanSetFocusAttribute); > + break; > > Add this same case to the updateIsolatedTree that takes a single > notification as opposed to a vector of notifications. We should refactor this such that we don't have to remember to add new notification handling in two places.
Created attachment 449231 [details] Patch
Committed r288058 (246078@main): <https://commits.webkit.org/246078@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449231 [details].