GNOME Bugzilla – Bug 746706
Serious accessible event spewage from Gtk+ table cells
Last modified: 2018-04-15 00:28:10 UTC
Created attachment 300225 [details] accessible-event listener Steps to reproduce: 1. Launch gtk3-demo's Tree Store demo 2. Launch the attached accessible-event listener in a terminal 3. Switch to the Tree Store demo window Expected result: Not 400+ accessible events from table cells Actual results: More than 400 (!!) accessible events from table cells: * object:state-changed:checked for the toggle-able cells * object:text-changed:insert for cells that display text * object:property-change:accessible-name for cells that display text What is triggering this event is examining the accessible tree (e.g. searching for objects with ROLE_LABEL). If you don't examine the tree, the spewage doesn't happen (at least not yet, and not to this extent). HOWEVER, it's not completely unreasonable for an AT to want to examine the accessible tree to search for a particular object, and doing so should not result in 400+ events. Happily this seems to be limited to table cells. And it looks like it might be on purpose: https://git.gnome.org/browse/gtk+/commit/?id=2e7fcc24ac45462ce2b75fd021e3e6fc2ad0479a Anyone recall why? Seems to me like a lot of unwanted noise (over dbus, for Orca to process enough to determine it's just spam, etc.)
Created attachment 300226 [details] event spewage resulting when performing the steps in the OR
Created attachment 300243 [details] [review] Eliminate signal spewage and fix some bugs in gtktextcellaccessible Sorry for the delay! I found more issues, including: 1. The deleted string was absent from the AT-SPI2 accessible event's any_data. 2. text-changed::delete was being emitted even if the "deleted" text was the empty string. 3. text-changed::insert was being emitted even if the "inserted" text was the empty string. The attached patch fixes those, along with stopping signal spewage when a new gtktextcellaccessible is created. I've tested it via pyatspi and Orca. So far so good. But GtkCellRenderer accessibility has been notoriously problematic. So.... A review from Benjamin or Matthias would be very much appreciated. :)
Okay, I couldn't help myself and split that commit up into its parts so I could understand that function and refactor it to make it actually look as simple as it is. I hope I didn't mess up anything along the way.
Thanks Benjamin! The other spewage (see the output I attached to comment 1) is state-changed events. That attachment just shows 'checked', but I'm guessing 'expanded' is in the same boat. So I'm going to re-open this bug and will hopefully have a patch for the state-changes tomorrow.
Seems something got unfixed along the way. I'll figure out what, but for now, we're back to cell-creation spewage (and the original problem from the OR). :(
Created attachment 300311 [details] [review] re-fix the cell-creation event flood Benjamin, what about this? (Feel free to trim down the comment. I admit it is kinda wordy/ranty. By the same token, documenting that this is a very intentional decision seems like it might be worth doing. IMHO.)
Looking at the other signals emitted upon creation which shouldn't be emitted.... I keep coming back to the following commit: https://git.gnome.org/browse/gtk+/commit/?id=2e7fcc24ac45 and wondering if we want to re-add the 'emit_change_signal' argument to the update_cache functions. Because what that commit undoes (at least part 1 of it) is the very thing (I think) we want to do, namely not emit events on newly-constructed cells. If the answer is "no", what alternative would you suggest to stop these floods?
Created attachment 301168 [details] [review] Only emit signals when the cell changes Given the absence of response to my question in comment 7, I would like to suggest we re-add an argument by which we can choose whether or not to emit signals on cells and attaching a patch to do so. I realize there is some disagreement about whose job it is to determine when change signals should be emitted. And perhaps we should have a full discussion about this. HOWEVER from a practical point of view, these floods need to stop. In that spirit: Gtk+ knows if the "change" is a real change (i.e. in an existing object) or a side effect (of setting up a new object whose name, state, whatever is something other than the default). Thus of all the modules whose job it might be to determine if ATs should receive this notification, Gtk+ is the best informed. :) Please review. Thanks!
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
*** Bug 726844 has been marked as a duplicate of this bug. ***
*** Bug 727256 has been marked as a duplicate of this bug. ***
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla. If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab: https://gitlab.gnome.org/GNOME/gtk/issues/new