GNOME Bugzilla – Bug 653293
AtkText: document new signals
Last modified: 2014-02-04 14:50:53 UTC
Created attachment 190546 [details] [review] patch ...and the old ones too.
*** Bug 653292 has been marked as a duplicate of this bug. ***
One thing I forgot to add: the docs should also state that 'empty' emissions with length == 0 should be suppressed.
Joanie points out that the @text parameter for text_insert/text_remove needs to be renamed to @object or so, to avoid a clash with the other @text.
I'll also point out that the new signals don't need to be G_SIGNAL_DETAILED, since they don't take any detail strings.
Ups, I missed this when I revamped ATK documentation with bug 684665. (In reply to comment #4) > I'll also point out that the new signals don't need to be G_SIGNAL_DETAILED, > since they don't take any detail strings. In any case I changed this with a recent commit. Closing bug. Thanks for the patch, and sorry for missing it.
This change broke the gtk+/a11y tests: gtk+/a11ychildren.test: (/usr/libexec/gtk+/installed-tests/children:7687): GLib-GObject-WARNING **: ../../gobject/gsignal.c:3110: signal id '286' does not support detail (363)
(In reply to comment #6) > This change broke the gtk+/a11y tests: > > gtk+/a11ychildren.test: (/usr/libexec/gtk+/installed-tests/children:7687): > GLib-GObject-WARNING **: ../../gobject/gsignal.c:3110: signal id '286' does not > support detail (363) My bad, I removed G_SIGNAL_DETAILED on the new signals as pointed by Matthias Clasen, but I wrongly removed it too on the old one. Fixed with this commit: https://git.gnome.org/browse/atk/commit/?id=693ff82e87471a2268fde236a0c97befe7d970c6 Thanks for pointing it.
Removing G_SIGNAL_DETAILED from text-insert also broke Firefox, because mozilla's a11y implementation emits the signal with detail. I agree this should be fixed in firefox, but not filling up everyone's log with GObject warnings is also a good idea...
(In reply to comment #8) > Removing G_SIGNAL_DETAILED from text-insert also broke Firefox, because > mozilla's a11y implementation emits the signal with detail. After a quick research. Firefox is adding the "::system" suffix here. So this bug is related to bug 639466 (initially assigned to Li Yuan, but that is not working on atk anymore). One of the problems on that bug was that was not possible to add two levels of details through at-spi. Now that with the new signals "insert" and "delete" detail is not needed, now it would be possible. So I will reopen this bug, and discuss a little with Joanmarie and Mike Gorse. Anyway right now I'm biased to the following solution: * Add again the flag G_SIGNAL_DETAILED for those events. * Document that one possible detail for those events is "::system" Note that document that "::system" on this signals will not automatically solve bug 639466, as that bug was wider, not only about this two signals (but will be a good starting point). > I agree this should be fixed in firefox, but not filling up everyone's log with > GObject warnings is also a good idea... Taking into account my previous comment, it is likely that we will solve this bug on ATK. Thanks for reporting this issue.
Created attachment 267944 [details] [review] adding support to details on text-insert and text-remove. Document it. After an IRC chat, implementation of what I suggested on comment 9. Posted in order to review documentation wording.
(In reply to comment #10) > Created an attachment (id=267944) [details] [review] > adding support to details on text-insert and text-remove. Document it. > > After an IRC chat, implementation of what I suggested on comment 9. Posted in > order to review documentation wording. Patch committted with some documentation nits (thanks Joanmarie for the review). Closing bug.
(In reply to comment #8) > Removing G_SIGNAL_DETAILED from text-insert also broke Firefox, because > mozilla's a11y implementation emits the signal with detail. > I agree this should be fixed in firefox, but not filling up everyone's log with > GObject warnings is also a good idea... FWIW, the fix for that problem was included on atk 2.11.6 release, just released today. Apologies for any inconvenience.