Re: [g-a-devel] FocuCaretTracker issues

[Piñeiro <apinheiro igalia com>]

Hi,

sorry for the delay answering. Some comment/questions below.


On 07/12/2013 10:33 PM, Joseph Scheuhammer wrote:
> Recently, Magdalen uploaded a patch to bugzilla
> (https://bugzilla.gnome.org/show_bug.cgi?id=647074#c16), that defined
> a mechanism whereby any gnome shell object could request notification
> of focus and/or caret change events as monitored by AT-SPI.
>
> It has been reviewed and that has led to a number of AT-SPI related
> questions.  These were discussed at this weeks #a11y-meeting, and I
> was asked to summarize them here.
>
> 1.  Should the tracker be a singleton?
> The tracker is currently implemented as a singleton, and is referenced
> through Main.  As such, it registers for the relevant AT-SPI events
> once.  Nonetheless, it allows multiple clients to request notification
> of those events.
>
> The alternative is for each client to create their own instance of the
> tracker.  In that case, each client registers for AT-SPI events, and
> it's likely that there will be multiple registrations for the same event.

In any case, another pov for the same question would be: is there any
advantage on each client having their own instance of the tracker?
Because if there isn't any advantage, having this as a singleton have
the memory advantage of maintaining just one reference. Probably that is
a small advantage, but if having one instance per client have zero
advantages, the singleton option is still winning 1-0.

> Mike Gorse is looking into whether AT-SPI event registration is
> limited to once-per-process, and may have answered on bugzilla by the
> time of this writing.

This is also something to take into account, so lets see if in the end
is a 2-0.

> 2. Where should Atspi.init() be done?
> This is equivalent to calling atspi_init() from libatspi.  Currently,
> nothing in gnome-shell calls it.  Since the tracker needs AT-SPI
> initialized, it calls it within its constructor, the logic being that
> if the tracker is needed, it will insure initialization.  Note that if
> AT-SPI is already initialized, calling init() again has no effect.
>
> Generally, there is reluctance to initialize accessibility if it is
> not needed.  This is to avoid DBUS messages unless an assistive
> technology is actually making use of AT-SPI.  The tracker's way of
> initializing AT-SPI is consistent with this reluctance.  The
> alternative is for AT-SPI to be initialized when gnome-shell is
> launched, most likely in  shell_a11y_init() in main.c.

Right now on shell_a11y_init the only thing initializated is the
atk-bridge. The GNOME developer community agreed to do the atk-bridge
initialization on each toolkit/app because Mike Gorse did the work to
avoid the DBUS emission from the bridge unless an AT is listening, and
some other improvements in order to avoid any "interference" on non-AT
users doing that initialization. AFAIK, there isn't any similar
isolation work on atspi. So:

 1. If calling ATSPI init doesn't affect users that not need the
tracker: then shell_a11y_init is a good place. And tidy, as all the
initialization is done at the same place.
 2. If calling ATSPI init can affect  users that not need the tracker:
then the init should be done somewhere (sorry for being vague) at the
code that starts the tracking focus if needed.

So now a proposal not really sourced (as I still didn't look at the
code, sorry). I assume that there are some code that stop the tracking
(so listening to atspi events), in the same way that it starts again the
tracking if needed. Lets say that is a setting "need-tracking".

So one option to merge 1. and 2. would be the next pseudocode.

shell_a11y_init {

if (need-tracking)
   initialize_atspi
}

need-tracking-setting-changed-callback {

   if (new_value == true) {
      initialize_atspi
    }

   if (new_value == false)
      clean_atspi_resources;
   }
}

As I said, not sure if it makes sense. If it is not probably I should
take a look to the code in order to have a proper opinion.

> 3. Should event registration happen in the tracker's constructor?
> Currently, the tracker does not register for the relevant AT-SPI
> events until at least one client has requested notification. Clients
> request such notification through a call to the tracker's connect()
> method.
>
> The alternative is for the tracker to register for all of the events
> in its constructor and be "prepped" to handle those events when a
> client connects.  (Note:  the three AT-SPI events in question are
> "object:state-changed:focus", "object:state-changed:select", and
> "object:caret-moved").

If my previous pseudocode make sense probably this question is also
answered. Probably replacing "need-tracking-setting-changed-callback"
for something like "client-has-requested-notification-callback".

> 4.  Is try/catch required when registering/deregistering with AT-SPI?
> Where the tracker registers/deregisters an event with AT-SPI, the code
> is wrapped within a try/catch block.  This is because when it was
> first written, registering/deregistering sometimes resulted in an
> exception.  However, these exceptions occurred in the same time frame
> as the "freeze problem"
> (https://bugzilla.gnome.org/show_bug.cgi?id=681276).  Since that
> freeze issue has been fixed, it may be that the try/catch block is now
> superfluous, and can be removed.
>
> Looking at the documentation, it appears that
> registering/deregistering does not throw an exception, and the
> try/catch can be removed:
> https://developer.gnome.org/libatspi/unstable/AtspiEventListener.html#atspi-event-listener-register

This is a question to Mike I fear. Sorry for not helping.

> 5.  Should clients have the opportunity to respond to focus events
> independent of select events?
> Currently, as far as a client is concerned, the tracker notifies them
> of focus changes.  However, on the AT-SPI side, the focus changes are
> represented by a combination of 'object:state-changed:focus" and
> "object:state-changed:select".  The standard example is arrowing
> through a menu:  the user perceives this as moving focus from menu
> item to menu item.  However, under the hood, there are no focus
> changes.  Rather, the system is emitting select changes.
>
> IMHO, as far as user experience is concerned select changes and focus
> changes are the same thing, aka perceived changes in focus.  I can't
> think of a use case where a client (AT) would want to track only
> "object:state-changed:focus" and not "object:state-changed:select", or
> vice versa.

I agree. If at this moment there is no use case to treat them
separately, treating that separately would be an overkill. In fact,
doing that is a example of a well known anti-pattern: "Accidental
complexity" [1]

[1] http://en.wikipedia.org/wiki/Accidental_complexity

Best regards

-- 
Alejandro Piñeiro Iglesias