Re: Comments on GTK+ patches from Atk tarball

[Owen Taylor <otaylor redhat com>]

Bill Haneman <Bill Haneman sun com> writes:

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

Yeah, its been awhile. :-(. Far too many balls in the air right
now.
 
> > * 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.  

What's the practical difference? The users will be calling methods
on different ATk interfaces, right?

> 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;

There are a lot of other things you might also want to add later...
Actually, extending this class to do anything useful will probably
break binary compatibility.

Can you expand a little on why you think you might want this class
in the future?

>  2) it is needed (or at least very helpful) for some of the several 
> approaches to custom widget support which we envision.

Can you describe these approaches?

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

All API has a cost:

 - Makes the documentation and system harder to grasp
 - May interfere with future changes
 - Slows things down / increases disk space (small for any one
   addition, but accumulates)

Once we add something to GTK+, it is, for all practical purposes
permanent. (Timescale for removing is a number of years.)

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

I think there really isn't a misunderstanding (or at least,
I figured out what you were doing at some point while writing
the mail), but rather, my gut instinct is that trying
to have your cake and eat it too for ref vs. get is wrong.
 
> > * 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.

As far as I see, you aren't mirroring the ATK interfaces into
GTK+. So, someone using ATK for acessiblity-enabling a GTK+ program
will be using large portions of the ATK interface.

As far as I understand it, ATK treats all accessibility objects potentially
as flyweights, and all navigation functions that return another
AtkObject return a new reference to that object?

If so, it seems that someone who wants to use ATK with GTK+ needs
to learn:

 AtkObjects are (potentially) flyweights and are ref'ed on return

While this may be painful, it's easier to learn than:

 AtkObjects are (potentially) flyweights and are ref'ed on return,
 except that there is a special AtkObject associated with each
 widget which is not a flyweight.

Have you thought more about how the new tree/list widgets fit
into the picture? 

[...]

> > * 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.  

The class is more extensible, if extensibility is needed. It also is
harder to remove if it turns out we needed something altogether
different.

I'm not horribly opposed to GtkAccessibileFactory, but it is different
than all the other places in GTK+ that we do something like this,
so I'm wary of it.

Wrapping function pointers as classes makes a lot of sense for Java,
and (to a lesser extent) C++. It makes less sense for C and for
languages like Perl, guile which have first-class function objects.

Basically, my feeling here is that the accessible-factory is something
that is only important to the accessibiltiy-enabling library.  As far
as a I can see, it isn't something you would use when implementing
your own custom widgets.

So, keeping the API here small and simple is more important to
me than keeping it cross-language, generic, extensible, etc.

> 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.

Could you explain what you mean by this?
 
> > * _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.

Not exporting it from the C file is enough emphasis for that...
 
> > * 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 you provide more background for these statements?

This seems to mostly be an implementation detail of the
accessibility-enabling library; 

And if I was writing a library in C to enable accessibility
in GTK+, I don't think I'd find creating a separate factory
class for each type of widget I wanted to enable particularly
convenient.

Regards,
                                        Owen