After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 751183 - No JAW_TYPE_WINDOW type
No JAW_TYPE_WINDOW type
Status: RESOLVED INCOMPLETE
Product: java-atk-wrapper
Classification: Applications
Component: general
unspecified
Other Linux
: High critical
: ---
Assigned To: Magdalen Berns (irc magpie)
java-atk-wrapper maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-06-18 20:56 UTC by Magdalen Berns (irc magpie)
Modified: 2015-07-02 13:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to address problem (9.81 KB, patch)
2015-06-18 21:02 UTC, Magdalen Berns (irc magpie)
committed Details | Review
patch to fix bug caused by 305627 (68.19 KB, patch)
2015-06-21 20:43 UTC, Magdalen Berns (irc magpie)
committed Details | Review
Patch to address comment 5 (2.74 KB, patch)
2015-07-02 13:12 UTC, Magdalen Berns (irc magpie)
committed Details | Review

Description Magdalen Berns (irc magpie) 2015-06-18 20:56:48 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.
Comment 1 Magdalen Berns (irc magpie) 2015-06-18 21:02:02 UTC
Created attachment 305627 [details] [review]
Patch to address problem
Comment 2 Magdalen Berns (irc magpie) 2015-06-21 20:43:11 UTC
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

  • #0 __GI_raise
    at ../sysdeps/unix/sysv/linux/raise.c line 55
  • #1 __GI_abort
    at abort.c line 89
  • #2 os::abort(bool)
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.45-39.b14.fc22.x86_64/jdk8/hotspot/src/os/linux/vm/os_linux.cpp line 1542
  • #3 VMError::report_and_die()
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.45-39.b14.fc22.x86_64/jdk8/hotspot/src/share/vm/utilities/vmError.cpp line 1094
  • #4 JVM_handle_linux_signal(int, siginfo_t*, void*, int)
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.45-39.b14.fc22.x86_64/jdk8/hotspot/src/os_cpu/linux_x86/vm/os_linux_x86.cpp line 541
  • #5 signalHandler(int, siginfo_t*, void*)
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.45-39.b14.fc22.x86_64/jdk8/hotspot/src/os/linux/vm/os_linux.cpp line 4268
  • #6 <signal handler called>
  • #7 g_int_hash
    at ghash.c line 1960
  • #8 g_hash_table_insert_internal
    at ghash.c line 375
  • #9 g_hash_table_insert_internal
    at ghash.c line 1226
  • #10 jaw_util_add_global_event_listener
    at jawutil.c line 348
  • #11 jaw_util_add_global_event_listener
    at jawutil.c line 136
  • #12 atk_add_global_event_listener
    at atkutil.c line 396
  • #13 add_signal_listener
    at event.c line 1164
  • #14 spi_atk_register_event_listeners
    at event.c line 1202
  • #15 spi_atk_activate
    at bridge.c line 942
  • #16 spi_atk_add_client
    at bridge.c line 1190
  • #17 impl_get_app_bus
    at application-adaptor.c line 105
  • #18 handle_message
    at droute.c line 553
  • #19 handle_message
    at droute.c line 600
  • #20 _dbus_object_tree_dispatch_and_unlock
    at dbus-object-tree.c line 1018
  • #21 dbus_connection_dispatch
    at dbus-connection.c line 4718
  • #22 message_queue_dispatch
    at atspi-gmain.c line 89
  • #23 g_main_context_dispatch
    at gmain.c line 3122
  • #24 g_main_context_dispatch
    at gmain.c line 3737
  • #25 g_main_context_iterate
    at gmain.c line 3808
  • #26 g_main_loop_run
    at gmain.c line 4002
  • #27 jni_loop_callback
    at AtkWrapper.c line 77
  • #28 g_thread_proxy
    at gthread.c line 764
  • #29 start_thread
    at pthread_create.c line 333
  • #30 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 109

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
Comment 3 Alejandro Piñeiro Iglesias (IRC: infapi00) 2015-06-22 10:16:10 UTC
(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
Comment 4 Alejandro Piñeiro Iglesias (IRC: infapi00) 2015-06-22 10:18:40 UTC
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.
Comment 5 Alejandro Piñeiro Iglesias (IRC: infapi00) 2015-06-22 10:23:28 UTC
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
Comment 6 Alejandro Piñeiro Iglesias (IRC: infapi00) 2015-06-22 10:30:18 UTC
(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.
Comment 7 Magdalen Berns (irc magpie) 2015-06-24 21:33:50 UTC
Review of attachment 305784 [details] [review]:

This was committed
Comment 8 Magdalen Berns (irc magpie) 2015-06-24 22:53:00 UTC
(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
Comment 9 Magdalen Berns (irc magpie) 2015-07-02 13:12:23 UTC
Created attachment 306616 [details] [review]
Patch to address comment 5