GNOME Bugzilla – Bug 577392
Gail doesn't emit children_changed::add when packing a box
Last modified: 2018-04-15 00:36:29 UTC
Please describe the problem: Gail does not emit a children_changed signal when a widget is inserted into a box using gtk_box_pack_start or gtk_box_pack_end. It relies on the widget sending an add signal to know when to emit children_changed, and add is not emitted when these functions are used. This causes trouble for the dbus port of at-spi, since it proactively transmits the hierarchy to the client for the most part and relies on signals to know when to update the cache. Steps to reproduce: 1. Start monitoring events, either with accerciser or with atspimon.py in the uia2atk repository. 2. Run the attached script. 3. Actual results: The first signal mentioning a PushButton gets emitted when one of the buttons receives the focus. Expected results: I would expect to see children-changed events when the buttons are added to the box. Does this happen every time? Yes. Other information: It doesn't appear that there is currently any signal that gail could listen to. I'll attach a possible patch.
Created attachment 131743 [details] Sample for reproducing the bug.
Created attachment 131744 [details] [review] Proposed patch.
Li, could you take a look at this patch?
Some one please review the gtk part of the patch first please?
::add is really just and implementation detail and not meant to be used like this. This problem is not limited to boxes; you have similar problems with menushells, tables, toolbars, etc. Also, for obvious symmetry reasons, adding ::child-added without a corresponding ::child-removed signal is a bit questionable. I think we probably want these signals in GtkContainer, if at all, and should probably look at emitting these whenever GtkWidget::parent-set is emitted. But gail could probably get away with just using ::parent-set itself. When that signal is emitted, you can access the new parent via gtk_widget_get_parent(), and the old parent (or NULL) is passed as an argument.
Created attachment 166507 [details] [review] Updated patch. I've followed Matthias's suggestion, and it worked with my test case at least. It changes the way children are initialized, though; parent_set is called on the widget being added, while add is called on the container to which the widget is added. Also, parent-set is apparently called earlier than add, since I initially got a crash since an accessible was being created for a widget that was not fully initialized, so I moved the initializations into an idle. Li, could you review? Is there a testing procedure that you would recommend? It feels like a significant change, so it would be good if there is a way to test it.
Created attachment 166548 [details] [review] Updated patch. Style fixes, and use a GQueue rather than a GList. This can wind up sending many more children-changed and property-change::accessible-parent events than before (when gedit starts up, for instance), which can affect performance. Still not sure how best to handle all of this.
Created attachment 231845 [details] [review] Patch to remove accessibles for hidden objects This patch removes accessibles for hidden objects, emitting children-changed signals when objects are hidden and only counting visible objects when enumerating children. Since it moves emission of children-changed signals to the point where the widget is shown, it also fixes this bug, so I'm adding a test for it. A consequence of removing accessibles on hiding of the widget is that they can go away before the widget does, so it is important that signals not remain on the widget that pass a pointer to the accessible as data without adding a reference on the pointer. In most of these cases, I've modified the gtk code to update the accessible directly. Sometimes the accessible was listening for a value-changed signal when the widget also sends out a notification when the value changes, so it looks as though it will suffice to send the notification in the accessible's notify_gtk function rather than adding a separate handler.
Created attachment 232695 [details] [review] Add gtk_notebook_accessible_get_n_children GtkNotebookAccessible is missing a get_n_children unction. Completely unrelated to this bug except that the test starts to fail without it.
Created attachment 232696 [details] [review] Patch to rework signal handlers to avoid dangling pointers.
Created attachment 232697 [details] [review] Patch to remove accessibles for widgets that are no longer visible. I've rebased my previous patch to apply against current gtk+ code and split off the signal changes into a separate patch.
Created attachment 233247 [details] [review] Updated patch. Add _gtk_range_accessible_value_changed, and call it rather than sending a notify on the accessible directly from gtkrange.c.
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.
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