search

AccessKit / accesskit

UI accessibility infrastructure across platforms and programming languages
BSD 3-Clause "New" or "Revised" License
991 stars 48 forks source link

fix: Return to handling focus events directly, after generic node changes #409

Closed mwcampbell closed 1 month ago

mwcampbell commented 1 month ago

I now believe that #354 was a mistake. So I've re-added the code in focus_moved to emit focus change events. But now, we specifically skip the focused state when looking for generic state changes. Reasons to do it this way:

  1. If a node has just been added, whether actually added to the tree or just newly visible in the filtered tree, we don't generally want to emit state change events for that node, but if it's the focused node, we do have to emit a focus event.
  2. If a node has other state changes at the same time that it receives focus, we want to emit the other state change events before the focus event. If we do it the other way around, as might happen depending on the order of the states, then Orca might read some states twice, once when presenting the newly focused node, and again in response to the state change event that was emitted after the focus change. I heard this happen; when leaving a menu and returning focus to the menu button, Orca would say something like, "Primary Menu, toggle button, not pressed. Not pressed."
  3. Also on the topic of event order, we should emit the activation event for the window before emitting the focus event for the node within the window. This change makes sure that it happens in that order.
DataTriny commented 1 month ago

@mwcampbell CI is either stuck here or its status report is bugged. Can you please rebase to trigger it again?

DataTriny commented 1 month ago

@mwcampbell The PR title doesn't make sense to me, and it's probably a bit vague to be included in the changelog. Could you please rename before merging?

mwcampbell commented 1 month ago

OK, is the new name better?

DataTriny commented 1 month ago

Yes. It still fails to mention that it only applies to the Unix adapter but it's quite long already. Feel free to merge as is if you can't rephrase it. It's acceptable now thanks.