Re: Comments on GTK+ patches from Atk tarball

[Bill Haneman <Bill Haneman sun com>]

Hi Owen:

Thanks for the feedback.  A couple of these points have come up before, 
I think, but it's been awhile :-)

Here are my responses, I think they reflect the design decisions made by 
me, Peter, and Padraig (Marc has newly joined us, and may have 
additional observations):

> * I don't see the point of GtkAccessible. As it stands now,
>   the only thing it has now is a backpointer to the widget
>   and no implementation. Is adding more stuff to this 
>   planned?
>
>   Basically, why have a publically visible gtk-namespaced AtkObject
>   derivative? People using the accessible functionality will be
>   referring to accessibles as AtkObject, I assume.

Actually, we anticipate that users of the "set" methods within GTK will 
refer to GtkAccessible instead of AtkObject.  At the moment, no, we only 
see the need for the back pointer, but the back pointer *is* important, 
the only question is whether this class should be visible to GTK+ or 
hidden in the implementation library.  

We see two reasons for exposing this class:
 1) we may need to add to it later, without breaking binary compat;
 2) it is needed (or at least very helpful) for some of the several 
approaches to custom widget support which we envision.

As for disadvantages, well, it doesn't seem harmful.

-----

I think there is a misunderstanding regarding get_accessible, as 
evidenced by the two comments below:

> * The fact that there is a public gtk_widget_get_accessible()
>   doesn't strike me as right. I would expect simply people
>   to use atk_object_ref_accessible (widget). [ There are
>   some naming problems in Atk with the interfaces, etc,
>   I'll comment on separately. ]

Within GTK+, the accessibles are *not* flyweights (unless they are 
returned from one of the "ref accessible children" calls).  The normal 
use within GTK+ and also within the implementation library is to use 
gtk_widget_get_accessible(), which does not return a flyweight and 
therefore relieves the caller of memory management worries.  The 
"ref_accessible" variant is there only to provide compatibility with the 
alternate, more generic, AtkAccessibleIface interface.  This more 
generic interface (which can be implemented on objects that are not 
GtkWidgets) will most likely only be used in the "bridge" code between 
ATK+ and the out-of-process SPI that is used by the assistive/adaptive 
technologies themselves.

>   And the pointer to the GtkAccessible in the GtkWidget structure
>   strikes me as odd; after all, the way we are handling 
>   accessibles is inherently treating them as flightweights.
>
>   I suppose what you are trying to do here is provide something
>   that conforms to the GTK+ conventions for naming and 
>   referencing return objects - I think this is perhaps
>   a false simplication; better to be conceptually simple
>   than to merely look simple.

See above: we are making a real distinction between the two entry points 
to this API, one requires refcount decrementing and the other (available 
only for GtkWidget instances/subclasses) does not.

> * While GtkAccessibleFactory makes sense, it strikes 
>   me that simply using a function pointer for this would
>   be simpler / easier. Using a class for this is (IMO)
>   overkill, and we certainly use function pointers througout
>   GTK+ for similar things.

Again, we think that using a class is more extensible.  It also means 
that we can implement reflection techniques on the factory instances 
after they have been associated with widget classes, which would be 
difficult if not impossible with function pointers.

> * _gtk_weaktype_ref_accessible shouldn't have the leading
>   underscore, since we don't use that for static functions,
>   and by GTK+ convention, would be simply gtk_widget_ref_accesible.

OK.  We just wanted to emphasize that this method is private.

> * The overhead of initializing every class in GTK+ is quite 
>   large and we intentionally delay initialization until an
>   instance of the class is created. So, I don't think
>  
>    gtk_widget_class_set_accessibility_factory()
> 
>   is a good idea; better to have a global factory.

There isn't a global factory, there are many factories.  Different 
factory instances must be associated with different classes, and 
changeable at runtime, thus the method above is required.

>Can I suggest something more like the following patch (not
>tested to compile):
>


Regards,

Bill

------
Bill Haneman x19279
Gnome Accessibility / Batik SVG Toolkit
Sun Microsystems Ireland