GNOME Bugzilla – Bug 751183
No JAW_TYPE_WINDOW type
Last modified: 2015-07-02 13:12:23 UTC
If the plan is to look up window signals from the window interface before emitting them then g_signal_lookup("create", JAW_TYPE_WINDOW) should be used which means that an appropriate header is actually needed and a class initialisation for it, after all.
Created attachment 305627 [details] [review] Patch to address problem
Created attachment 305784 [details] [review] patch to fix bug caused by 305627 Many apologies, but I have to reopen this bug because the implementation I used caused bug 751159. Unfortunately, bug 751159 seems to have got missed at first, presumably because of some stray left over old library files never got cleaned by 'make distclean' as would be expected after having changed the build configuration (and failing to make the necessary manual checks at that time). Although the window events seemed to be looked up okay after some tweaking, some of the changes seemed to cause a failure on GTK:AtkObject:property-change. I have no idea why yet but here's a stack-trace for future reference
+ Trace 235196
I looked over whether this should be implemented as a jaw class or a jaw interface or both and some things to note are: * There is no AccessibleWindow class or anything like it in javax/accessibility * There is no ATK window adapter[1] So anyway, I scratched my head over this problem all weekend but ultimately, I had to accept that this is not actually an urgent update and that patches which followed need to be rechecked now anyway so I took the decision to revert to the state before this patch re-apply working changes and run the necessary checks for them making sure any old libraries have been cleaned up. I will therefore now re-open this bug at a low the priority and will try to be a lot more careful about cleaning up before testing next time build directories get changed! [1] https://git.gnome.org/browse/at-spi2-atk/tree/atk-adaptor/adaptors
(In reply to Magdalen Berns (irc magpie) from comment #0) Hi, I took a brief look to the code, so although I don't have a counter-patch, I have some suggestions. I hope those could help. > If the plan is to look up window signals from the window interface before > emitting them then g_signal_lookup("create", JAW_TYPE_WINDOW) should be used > which means that an appropriate header is actually needed and a class > initialisation for it, after all. I don't think that adding a JAW_TYPE_WINDOW would be the solution. Looking at the JAW code, there isn't specific JAW types for each ATK component. So there isn't a JAW_TYPE_ACTION, JAW_TYPE_IMAGE, etc. There are JAW_TYPE_OBJECT, JAW_TYPE_HYPERLINK, JAW_TYPE_UTIL, but those are subclasses of specific ATK classes, not interfaces. So as far as I see, jaw has a wrapper object of type JAW_TYPE_OBJECT, that is a subclass of AtkObject. And the way to deal with specific Atk interfaces is by creating an instance of JAW_TYPE_OBJECT implementing that interface by demand. This instance is created at jaw_impl_get_instance [1], that calls aggregate_interface[2]. Aggregate interface add atk interfaces to the object, checking if it is needed or not (if it is a component, it adds atk_component, etc). What it is missing right now is managing the window interface. So in summary, instead of adding a new type JAW_TYPE_WINDOW, what it would be needed is managing the window interface at agregate_interface. [1] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawimpl.c#n254 [2] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawimpl.c#n145
Review of attachment 305627 [details] [review]: ::: jni/src/jawwindow.c @@ +39,3 @@ + JAW_TYPE_WINDOW, + G_IMPLEMENT_INTERFACE (ATK_TYPE_WINDOW, + jaw_window_interface_init)); As mentioned in my previous comment, it is not needed (and in fact probably is just wrong) to create a new type. Instead of using as reference what the support of AtkHyperlink is doing (as AtkHyperlink is not an interface, but a new defined subclass of GObject), probably it would be good if you use as reference what jawcomponent is doing, as AtkComponent is an interface, like AtkWindow.
Review of attachment 305627 [details] [review]: Sorry for the SPAM, I forgot to mention something before. ::: jni/src/AtkWrapper.c @@ +320,3 @@ NULL); + g_signal_emit(atk_obj, g_signal_lookup("create", JAW_TYPE_WINDOW), 0); I think that the code would be more easy to understand if instead of this, you use g_signal_emit_by_name. So something like : g_signal_emit_by_name(atk_obj, "create", 0) As far as atk_obj implements AtkWindow (and that would be solved if window is managed at aggregate_interface), that should work. This is how jaw emit other signals, for example, on the case of one AtkText signal: https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/AtkWrapper.c#n797
(In reply to Magdalen Berns (irc magpie) from comment #2) > * There is no ATK window adapter[1] > [1] https://git.gnome.org/browse/at-spi2-atk/tree/atk-adaptor/adaptors There is no atk window adapter at at-spi2-atk because it is not needed. There isn't anything to adapt. Right now AtkWindow doesn't define any method, that is the purpose of the adaptors. It just defines the interface and some signals. Signal forwarding is managed here: https://git.gnome.org/browse/at-spi2-atk/tree/atk-adaptor/event.c > So anyway, I scratched my head over this problem all weekend but ultimately, > I had to accept that this is not actually an urgent update and that patches > which followed need to be rechecked now anyway so I took the decision to > revert to the state before this patch re-apply working changes and run the > necessary checks for them making sure any old libraries have been cleaned up. Not sure for the case of JAVA applications, but in the case of GNOME shell, if window signals are lacking, Orca is not able to know that gnome shell became the active application.
Review of attachment 305784 [details] [review]: This was committed
(In reply to Alejandro Piñeiro Iglesias (IRC: infapi00) from comment #3) > (In reply to Magdalen Berns (irc magpie) from comment #0) > > Hi, I took a brief look to the code, so although I don't have a > counter-patch, I have some suggestions. I hope those could help. Thanks they do, I committed the revert because it fixed the problem quickly and everything since would have needed to be re-checked anyway, but I would really like to understand this better. > > > If the plan is to look up window signals from the window interface before > > emitting them then g_signal_lookup("create", JAW_TYPE_WINDOW) should be used > > which means that an appropriate header is actually needed and a class > > initialisation for it, after all. > > I don't think that adding a JAW_TYPE_WINDOW would be the solution. Looking > at the JAW code, there isn't specific JAW types for each ATK component. So > there isn't a JAW_TYPE_ACTION, JAW_TYPE_IMAGE, etc. There are > JAW_TYPE_OBJECT, JAW_TYPE_HYPERLINK, JAW_TYPE_UTIL, but those are subclasses > of specific ATK classes, not interfaces. That makes sense. > So as far as I see, jaw has a wrapper object of type JAW_TYPE_OBJECT, that > is a subclass of AtkObject. And the way to deal with specific Atk interfaces > is by creating an instance of JAW_TYPE_OBJECT implementing that interface by > demand. This instance is created at jaw_impl_get_instance [1], that calls > aggregate_interface[2]. Aggregate interface add atk interfaces to the > object, checking if it is needed or not (if it is a component, it adds > atk_component, etc). What it is missing right now is managing the window > interface. > > So in summary, instead of adding a new type JAW_TYPE_WINDOW, what it would > be needed is managing the window interface at agregate_interface. > > > [1] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawimpl.c#n254 > [2] https://git.gnome.org/browse/java-atk-wrapper/tree/jni/src/jawimpl.c#n145 Ah right, I will look into this. (In reply to Alejandro Piñeiro Iglesias (IRC: infapi00) from comment #4) > Review of attachment 305627 [details] [review] [review]: > > ::: jni/src/jawwindow.c > @@ +39,3 @@ > + JAW_TYPE_WINDOW, > + G_IMPLEMENT_INTERFACE (ATK_TYPE_WINDOW, > + jaw_window_interface_init)); > > As mentioned in my previous comment, it is not needed (and in fact probably > is just wrong) to create a new type. > > Instead of using as reference what the support of AtkHyperlink is doing (as > AtkHyperlink is not an interface, but a new defined subclass of GObject), > probably it would be good if you use as reference what jawcomponent is > doing, as AtkComponent is an interface, like AtkWindow. (In reply to Alejandro Piñeiro Iglesias (IRC: infapi00) from comment #6) > (In reply to Magdalen Berns (irc magpie) from comment #2) > > > * There is no ATK window adapter[1] > > > [1] https://git.gnome.org/browse/at-spi2-atk/tree/atk-adaptor/adaptors > > There is no atk window adapter at at-spi2-atk because it is not needed. > There isn't anything to adapt. Right now AtkWindow doesn't define any > method, that is the purpose of the adaptors. It just defines the interface > and some signals. Signal forwarding is managed here: > https://git.gnome.org/browse/at-spi2-atk/tree/atk-adaptor/event.c This is all very useful stuff. Thanks. > > So anyway, I scratched my head over this problem all weekend but ultimately, > > I had to accept that this is not actually an urgent update and that patches > > which followed need to be rechecked now anyway so I took the decision to > > revert to the state before this patch re-apply working changes and run the > > necessary checks for them making sure any old libraries have been cleaned up. > > Not sure for the case of JAVA applications, but in the case of GNOME shell, > if window signals are lacking, Orca is not able to know that gnome shell > became the active application. That's okay, the window signals are firing but I am not seeing all of them which is something I need to investigate further. I'm not at the other computer to double check at this minute but I think it's "activate" and, either "destroy" or "deactivate", which are ok with SwingSet2. The java signals don't directly "translate" though[1,2] [1] http://docs.oracle.com/javase/7/docs/api/java/awt/event/WindowEvent.html [2] https://developer.gnome.org/atk/unstable/AtkWindow.html
Created attachment 306616 [details] [review] Patch to address comment 5