GNOME Bugzilla – Bug 751267
Support the use of suffixes for signals in jaw_add_global_event_listener()
Last modified: 2015-07-07 19:41:21 UTC
In a similar was as has been done to resolve the issue in bug 686801 this is also an issue for the global jaw listener in jawutil.c.[1] [1] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawutil.c#n276
(In reply to Magdalen Berns (irc magpie) from comment #0) > In a similar was as has been done to resolve the issue in bug 686801 this is > also an issue for the global jaw listener in jawutil.c.[1] > > [1] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawutil.c#n276 As far as I see that method is just to be used here: https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawutil.c#n118 and that one is mostly a C&P from what gtk+ or clutter had in the past. After the addition of the AtkWindow interface, it was concluded that, in general, it was not needed a custom per-atk-implementor implementation of atk_add_global_event_listener (that also applies to atk_remove_global_event_listener). So a default implementation was added on atk, and the custom implementation started to be removed from several toolkits. That is properly explained on atk_add_global_event_listener documentation: https://developer.gnome.org/atk/unstable/AtkUtil.html#atk-add-global-event-listener Specifically: "Toolkit implementor note: ATK provides a default implementation for this virtual method. ATK implementors are discouraged from reimplementing this method." So my suggestion is that JAW custom implementation should be also removed. As a reference, the following links show the links of that removal on gtk+ and clutter: https://git.gnome.org/browse/gtk+/commit/?id=4cfe2a38bf32520001b9836433915713d248f04a https://git.gnome.org/browse/clutter/commit/?id=a8c829019f6d659489d1155ca545b38837b105ea
Created attachment 307027 [details] [review] JNI: remove custom implementation of util listeners (In reply to Alejandro Piñeiro Iglesias (IRC: infapi00) from comment #1) > (In reply to Magdalen Berns (irc magpie) from comment #0) > > In a similar was as has been done to resolve the issue in bug 686801 this is > > also an issue for the global jaw listener in jawutil.c.[1] > > > > [1] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawutil.c#n276 > > As far as I see that method is just to be used here: > https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawutil.c#n118 > > and that one is mostly a C&P from what gtk+ or clutter had in the past. > > After the addition of the AtkWindow interface, it was concluded that, in > general, it was not needed a custom per-atk-implementor implementation of > atk_add_global_event_listener (that also applies to > atk_remove_global_event_listener). So a default implementation was added on > atk, and the custom implementation started to be removed from several > toolkits. > > That is properly explained on atk_add_global_event_listener documentation: > https://developer.gnome.org/atk/unstable/AtkUtil.html#atk-add-global-event- > listener > Specifically: "Toolkit implementor note: ATK provides a default > implementation for this virtual method. ATK implementors are discouraged > from reimplementing this method." > > So my suggestion is that JAW custom implementation should be also removed. > As a reference, the following links show the links of that removal on gtk+ > and clutter: > https://git.gnome.org/browse/gtk+/commit/ > ?id=4cfe2a38bf32520001b9836433915713d248f04a > https://git.gnome.org/browse/clutter/commit/ > ?id=a8c829019f6d659489d1155ca545b38837b105ea OK, thanks for the advice, explanation and references. I had wondered why these implementors weren't there in other examples so this has cleared that up too. I have not pushed the proposed patch, since I want to test this quite carefully first. At the moment, the wrapper still has not got shot of jaw_window_signals which seem to use this.[1] I'll have another look at the discussion on bug 751183 about that if I find any problems with it. [1] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawobject.c#n62
Review of attachment 307027 [details] [review]: Great thanks again for your help with this Alejandro. I have had a chance to triple check this a few times now and it does seem to work well (with the added bonus that it also seems to fix bug 751053) so I have pushed it to master.