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 638924 - Implement AtkWindow
Implement AtkWindow
Status: RESOLVED FIXED
Product: atk
Classification: Platform
Component: atk
unspecified
Other Linux
: Normal normal
: ---
Assigned To: ATK maintainer(s)
ATK maintainer(s)
Depends on:
Blocks: 638537 649577 656306
 
 
Reported: 2011-01-07 17:03 UTC by Alejandro Piñeiro Iglesias (IRC: infapi00)
Modified: 2011-08-16 17:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch. (6.00 KB, patch)
2011-07-07 21:41 UTC, Mike Gorse
needs-work Details | Review
Updated patch. (5.61 KB, patch)
2011-07-09 05:08 UTC, Mike Gorse
none Details | Review
Adding atkwindow on different places (1.50 KB, patch)
2011-08-10 18:38 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
committed Details | Review
Updated patch. (5.66 KB, patch)
2011-08-16 16:48 UTC, Mike Gorse
committed Details | Review

Description Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-01-07 17:03:23 UTC
at-spi expect window related methods (activate, deactivate, etc) from the ATK implementation toolkit, defined on the interface Window, at event.didl (at-spi2-core/idl).

But the equivalent methods are not defined on ATK. That means that each implementation are emitting it as a private detail of the implementation, but not related with any Atk event.

This also leads to hacky implementation of the method atk-add-global-event-listener, as it requires to provide the way to add a listener to ATK methods, and also this "out the ATK-law" methods.

In my opinion that could be solved with an AtkWindow interface, with an API more or less equivalent to the defined on that at-spi interface Window.
Comment 1 Joanmarie Diggs (IRC: joanie) 2011-05-11 08:34:30 UTC
ATK Hackfest Conclusions:

* We will add this interface
Comment 2 André Klapper 2011-06-23 22:06:38 UTC
[Mass-reassigning open atk bug reports for better trackability as requested in https://bugzilla.gnome.org/show_bug.cgi?id=653179 .
PLEASE NOTE:
If you have watched the previous assignee of this bug report as a workaround for actually getting notified of changes in atk bugs, you yourself will now have to add atk-maint@gnome.bugs to your watchlist at the bottom of https://bugzilla.gnome.org/userprefs.cgi?tab=email to keep watching atk bug reports in GNOME Bugzilla.
Sorry for the noise: Feel free to filter for this comment in order to mass-delete the triggered bugmail.]
Comment 3 Mike Gorse 2011-07-07 21:41:57 UTC
Created attachment 191498 [details] [review]
Patch.

Tested with gtk+ master after removing the window signal registrations.

This patch seems disruptive to cally, but I'm not sure why--I haven't looked at the code or tried to debug.  My inclination would be to commit it to atk-3.

Also, I haven't added entries for the signals in the structure, as has generally been done for other ATK interfaces; let me know if you think these should be added.
Comment 4 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-07-08 12:13:14 UTC
(In reply to comment #3)
> Created an attachment (id=191498) [details] [review]
> Patch.
> 
> Tested with gtk+ master after removing the window signal registrations.

Did you also updated the signal emission?

> This patch seems disruptive to cally, but I'm not sure why--I haven't looked at
> the code or tried to debug.  My inclination would be to commit it to atk-3.

Ok, I will test it on cally. Anyway, once we get those thing solved, I think that it would be save to add this interface to atk-2 itself.

> Also, I haven't added entries for the signals in the structure, as has
> generally been done for other ATK interfaces; let me know if you think these
> should be added.

No, the current "policy" is avoid that unless it is really required.

Review in progress.
Comment 5 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-07-08 12:30:03 UTC
Review of attachment 191498 [details] [review]:

::: atk/atkwindow.c
@@ +1,3 @@
+/* ATK -  Accessibility Toolkit
+ * Copyright 2001 Sun Microsystems Inc.
+ *

As I said, date and company seems to no be the correct.

@@ +57,3 @@
+  return type;
+}
+

How about use G_DEFINE_INTERFACE?

Also related with bug 382323

::: atk/atkwindow.h
@@ +1,2 @@
+/* ATK -  Accessibility Toolkit
+ * Copyright 2001 Sun Microsystems Inc.

Probably you would prefer to use a different copyright here.

@@ +40,3 @@
+
+#ifndef _TYPEDEF_ATK_WINDOW_
+#define _TYPEDEF_ATK_WINDOW_

I saw this ifndefs in the other atk interfaces, but it is not used on gtk and clutter. It is really required?

@@ +52,3 @@
+  gpointer       pad2;
+  gpointer       pad3;
+  gpointer       pad4;

At this moment clutter does something like:

  gpointer _padding_dummy[28];

I think that this make things more readable.
Comment 6 Mike Gorse 2011-07-09 04:53:47 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Created an attachment (id=191498) [details] [review] [details] [review]
> > Patch.
> > 
> > Tested with gtk+ master after removing the window signal registrations.
> 
> Did you also updated the signal emission?
> 
I haven't done anything to the signal emission so far--it is calling g_signal_lookup on g_signal_lookup on GTK_TYPE_WINDOW_ACCESSIBLE.  It works if I implement AtkWindow but not if I don't implement AtkWindow.
Comment 7 Mike Gorse 2011-07-09 05:08:42 UTC
Created attachment 191554 [details] [review]
Updated patch.

Thanks for the review.  I copied that typedef over from another file; I don't know why it was there.  I took it out, and it builds for me.
Comment 8 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-10 18:38:55 UTC
Created attachment 193579 [details] [review]
Adding atkwindow on different places

I guess that atk.symbols will be required for windows.

I needed to add AtkWindow on atknoopobject because the bridge uses it to ensure that we have the gtype available for all the atk interfaces.

And after all, it is supposed that that object implements all the interfaces.
Comment 9 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-10 18:55:36 UTC
> This patch seems disruptive to cally, but I'm not sure why--I haven't looked at
> the code or tried to debug.  My inclination would be to commit it to atk-3.

Take a look to bug 649577. That "disruption" was related to the tests compiling
or when using it?

Although I still don't know why failed the test compilation, once solved I was
able to test it using cally-atkkeyevents example, at tests/accessibility.

Of course that will  not work with the atk-bridge until a equivalent change to
that example is made (registering window events with something like
"Atk:AtkWindow:create" instead of the current "window:create").

Any other reason to postpone it to ATK3?

> Also, I haven't added entries for the signals in the structure, as has
> generally been done for other ATK interfaces; let me know if you think these
> should be added.

No, I'm ok with that.

In the same way I just added a little patch with some minor issues.

For me the patch is ok. But I will approve it once we get answered those
questions.
Comment 10 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-15 14:58:14 UTC
(In reply to comment #9)
> > This patch seems disruptive to cally, but I'm not sure why--I haven't looked at
> > the code or tried to debug.  My inclination would be to commit it to atk-3.
> 
> Take a look to bug 649577. That "disruption" was related to the tests compiling
> or when using it?

Sorry typo:

https://bugzilla.gnome.org/show_bug.cgi?id=656306#c4

Mike, had you got this error?
Comment 11 Mike Gorse 2011-08-16 16:48:37 UTC
Created attachment 193977 [details] [review]
Updated patch.

Fix copyright.