Make m_deferredFocusedNodeChange a single item rather than a list to make AXObjectCache::handleFocusedUIElementChanged easier to reason about
<rdar://problem/105950128>
Created attachment 465186 [details] Patch
Comment on attachment 465186 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=465186&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:1380 > + m_deferredFocusedNodeChange->second = newNode; Do we need to handle case where Old = a New = b Then 2nd Old = b New = a Do we avoid any work here?
(In reply to chris fleizach from comment #3) > Comment on attachment 465186 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=465186&action=review > > > Source/WebCore/accessibility/AXObjectCache.cpp:1380 > > + m_deferredFocusedNodeChange->second = newNode; > > Do we need to handle case where > > Old = a > New = b > > Then 2nd > Old = b > New = a > > Do we avoid any work here? Ah, good point. I would interpret that sequence as no focus change happening (because we never were actually able to commit `b` as the new focused element before `a` regained focus). I'll update the patch.
Created attachment 465188 [details] Patch
Created attachment 465189 [details] Patch
Created attachment 465193 [details] Patch
(In reply to Tyler Wilcock from comment #7) > Created attachment 465193 [details] > Patch --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp // Recompute isIgnored after a focus change in case that altered visibility. - recomputeIsIgnored(deferredFocusedChangeContext.first); - recomputeIsIgnored(deferredFocusedChangeContext.second); + recomputeIsIgnored(m_deferredFocusedNodeChange->first.get()); + recomputeIsIgnored(m_deferredFocusedNodeChange->second.get()); in the case of a -> b -> c, in rapid submission, you end up with (a, c), so you never do above for b. Would that have any side effect? --- a/LayoutTests/accessibility/ios-simulator/focus-change-notifications.html +++ b/LayoutTests/accessibility/ios-simulator/focus-change-notifications.html +var rootElement = 0; +var rootElement = null; While you're at this test, can we modernize it?
Created attachment 465222 [details] Patch
(In reply to Tyler Wilcock from comment #9) > Created attachment 465222 [details] > Patch --- a/LayoutTests/accessibility/ios-simulator/focus-change-notifications.html +++ b/LayoutTests/accessibility/ios-simulator/focus-change-notifications.html +successfullyParsed = true; I don't think this variable is needed at all.
Created attachment 465224 [details] Patch
Created attachment 465229 [details] Patch
Created attachment 465231 [details] Patch
Created attachment 465232 [details] Patch
Committed 261037@main (c87b5bf70b93): <https://commits.webkit.org/261037@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 465232 [details].