Re: [g-a-devel] RFC: AtkText simplification (take 3)

[Trevor Saunders <trev saunders gmail com>]

On Fri, Aug 09, 2013 at 11:26:41AM +0200, Piñeiro wrote:
> On 08/06/2013 10:11 PM, Trevor Saunders wrote:
> > On Tue, Aug 06, 2013 at 04:22:40PM +0200, Mario wrote:
> > > Hi Trevor,
> > >
> > > On 6 August 2013 16:05, Trevor Saunders <trev saunders gmail com> wrote:
> > > > > 6. In atspi2-atk bridge, check which version of ATK a specific
> > > > > application is implementing (using atk_get_version()) when
> > > > > implementing this new atk_text_get_text_for_offset(), so we know
> > > > > whether we can call atk_text_get_text_for_offset() or we need to use
> > > > > the old atk_text_get_text_at_offset() and a *_START boundary instead
> > > > > (for implementors of older versions of ATK).
> > > > afaik atk_get_version() only tells you what version of libatk-1.0.so you
> > > > are talking to not what version of the atk headers an application was
> > > > compiled against.  So I believe that would cause breakage when people
> > > > compile applications against atk pre this change and then run that
> > > > application with a newer libatk-1.0.so.
> > > Yes you are right. We already realized a while ago this part was wrong
> > > and were already thinking that a better approach would be to use
> > > ATK_CHECK_VERSION() to know whether we need to use the old API or if
> > > we can use the new one (and maybe with a fallback to the old API in
> > > case the app that is running has not been updated yet to it and it's
> > > still implementing the old functions).
> > Well, what's important is the version of atk that the application was
> > compiled against not what version of atk you're compiling atk-bridge
> > against. 
>
> Both are important. In order to use the new method, atk-bridge needs to
> compile and link against a version of atk that contains the new method.
> And atk will provide a dummy implementation of that method, so in fact
> they can call it.
>
> In any case, you are right in something. ATK_CHECK_VERSION will inform
> you about the atk version the bridge is compiling against, not the one
> used by the implementor. So the only advantage of using
> ATK_CHECK_VERSION is that you don't need to update the dependency of the
> atk-bridge. And taking into account that this is the atk bridge, seems
> like an overkill. Using ATK_CHECK_VERSION makes sense on projects like
> WebKitGTK where there are several dependencies, and ATK is "just one
> more", but in at-spi-atk, atk is one of the more important ones, so
> updating the atk version needed is what makes sense, imho. I will
> mention that on the patch review.

yes, this is what I meant when I said only the other part was
important.

> >  So about the only way I can think of to know what version of
> > atk a an application was compiled with given only its binary is to see
> > if it defines a symbol and then arrange to have atk define this symbol
> > at the same time you add this new virtual function.  
>
> The general idea is what Frederik proposed on an ATK/AT-SPI hackfest,
> providing a way to know if the current implementation is providing a
> feature or not. Anyway, your implementation proposal seems somewhat
> intrusive, and in fact ...
>
> > However having your
> > headers inject symbols feels somewhat rude.  
> ... it seems that you agree with that.

correct

> > I *believe* the symbol
> > approach will work, but changing vtables and not changing abi feels
> > dangerous to me and I would try to avoid it unless I had no other
> > reasonable option.
>
> I think that in the end we shouldn't worry too much about this in this
> scope. I would maintain the temporal wrapper to the old method for this
> bug (fwiw, bgo#705581) and think about the other problem from a more
> general pov. I wouldn't block this patch for that issue.

so its unclear to me how you do intend to decide if you need to use the
old method now.  However if you don't intend to break the abi you need
to be sure the code that defines the atkobject you're dealing with was
compiled against atk headers that defined the new method or you could
end up reading some uninitialized memory and interpreting it as a
function pointer.

Trev

> gnome-accessibility-devel gnome org
> https://mail.gnome.org/mailman/listinfo/gnome-accessibility-devel