per irc chat with Joanie and API: it's deprecated and it fires deprecated focus event alexsurkov: ok, so when we handled focus internally then all we should do is to fire state change event for focus state and that's all, right? [11:07am] joanie: right API: so in summary, now we are in the process to: [11:09am] API: 1. orca removing support of focus event [11:10am] API: 2. after that, ping toolkits/apps to stop to use trackers and emit atkobject:focus Also (we fire focus event when menu appears): alexsurkov: joanie: when menu is shown then we fire visible/showing state change plus a focus event. Do you need any focus notification in this case? [11:16am] alexsurkov: (I realise when menu is shown then some item gets focused anyway) [11:16am] joanie: for menus we're switching over to selection

FWIW, the list of all the focus related methods and events deprecated are explained here: https://bugzilla.gnome.org/show_bug.cgi?id=649575#c8

Comment on attachment 828089 [details] [diff] [review] patch > // Fire state change event for focus > nsRefPtr<AccEvent> stateChangeEvent = > new AccStateChangeEvent(accessible, states::FOCUSED, true); > return FireAtkStateChangeEvent(stateChangeEvent, atkObj); that's amazingly inefficient want to file a gfb to just inline the relevent line of FireStateChangeEvent() ?

(In reply to Alejandro Piñeiro from comment #1) > FWIW, the list of all the focus related methods and events deprecated are > explained here: > https://bugzilla.gnome.org/show_bug.cgi?id=649575#c8 I'm assuming nobody ever used this stuff for anything? If somebody did use them at some point then we can't do this and need to either just keep firing these events even if they're useless or do runtime checks for atk version and hope that correlates with consumers going away.

(In reply to Trevor Saunders (:tbsaunde) from comment #2) > that's amazingly inefficient want to file a gfb to just inline the relevent > line of FireStateChangeEvent() ? I filed bug 935756, I hope it's ok I pointed you as a mentor

(In reply to Trevor Saunders (:tbsaunde) from comment #3) > (In reply to Alejandro Piñeiro from comment #1) > > FWIW, the list of all the focus related methods and events deprecated are > > explained here: > > https://bugzilla.gnome.org/show_bug.cgi?id=649575#c8 > > I'm assuming nobody ever used this stuff for anything? If somebody did use > them at some point then we can't do this and need to either just keep firing > these events even if they're useless or do runtime checks for atk version > and hope that correlates with consumers going away. Well, there is a long description of all that on the bug description itself [1], but I will summarize it. About the following methods deprecated: atk_add_focus_tracker atk_remove_focus_tracker atk_focus_tracker_init atk_focus_tracker_notify atk_component_add_focus_handler atk_component_remove_focus_handler Those were used, but by the implementors. There were utility methods used to solve the problem "know which is the focused object, notify when the focused object changes". And some implementors are implementing/using them. But an AT doesn't need them to be used by the ATK implementor. The only thing that you need is getting that problem in quotes solved. The main reason to deprecate them was that they were not so useful as utility methods, as toolkits/apps usually have better ways to know when the focus changed. Using them led to a messy implementation, and having so many methods on the ATK API lead to a messy API. And we didn't deprecated them without testing first. Taking into account that clutter (used by gnome-shell) ATK implementation was small, I tested first there. I stopped to use those methods, and just emitted the signal when needed. You can see the change here [2]. IMHO, the code that deal with the change of the focused event became clearer after the change. In summary: stop to use all those methods are safe from the POV of any version of an AT. So the other element deprecated: AtkObject::focus-event Yes it is true that this event was used by Orca. In fact, it is still used, as Joanie is modifying Orca in order to only take care of atkobject:state-changed:focused. And it is true that if firefox stop to emit AtkObject::focus-event, and the user is using an old version of Orca, it would not receive those a event is listening too. But we didn't see that as a big problem. Right now, it is really usual that each Firefox update needs a Orca tweak. Probably you are already aware due this bug [4]. Another example here [3]. So a recent Firefox needs a recent Orca. In any case, probably a compromise would be keep emitting AtkObject::focus-event if you are worried about old versions of Orca. Newer versions of Orca will just ignore it. [1] https://bugzilla.gnome.org/show_bug.cgi?id=649575#c0 [2] https://git.gnome.org/browse/clutter/commit/?id=cc126f55eb948a528211bc1649cd20bc7a7c0ed7 [3] https://mail.gnome.org/archives/orca-list/2013-November/msg00047.html [4] https://bugzilla.mozilla.org/show_bug.cgi?id=935583

It seems API's answer should clear a need-info request

(In reply to alexander :surkov from comment #7) > It seems API's answer should clear a need-info request no, see below. (In reply to Alejandro Piñeiro from comment #5) > (In reply to Trevor Saunders (:tbsaunde) from comment #3) > > (In reply to Alejandro Piñeiro from comment #1) > > > FWIW, the list of all the focus related methods and events deprecated are > > > explained here: > > > https://bugzilla.gnome.org/show_bug.cgi?id=649575#c8 > > > > I'm assuming nobody ever used this stuff for anything? If somebody did use > > them at some point then we can't do this and need to either just keep firing > > these events even if they're useless or do runtime checks for atk version > > and hope that correlates with consumers going away. > > Well, there is a long description of all that on the bug description itself > [1], but I will summarize it. > > About the following methods deprecated: > atk_add_focus_tracker > atk_remove_focus_tracker > atk_focus_tracker_init > atk_focus_tracker_notify > atk_component_add_focus_handler > atk_component_remove_focus_handler > > Those were used, but by the implementors. There were utility methods used to > solve the problem "know which is the focused object, notify when the focused > object changes". And some implementors are implementing/using them. But an > AT doesn't need them to be used by the ATK implementor. The only thing that > you need is getting that problem in quotes solved. The main reason to > deprecate them was that they were not so useful as utility methods, as > toolkits/apps usually have better ways to know when the focus changed. Using > them led to a messy implementation, and having so many methods on the ATK > API lead to a messy API. > > And we didn't deprecated them without testing first. Taking into account > that clutter (used by gnome-shell) ATK implementation was small, I tested > first there. I stopped to use those methods, and just emitted the signal > when needed. You can see the change here [2]. IMHO, the code that deal with > the change of the focused event became clearer after the change. that's great but its not really related to my question ;) > In summary: stop to use all those methods are safe from the POV of any > version of an AT. ok, afaik we basically weren't using them anyway (we just used the atk_notify_focus_tracker thing to send the event below). > So the other element deprecated: > AtkObject::focus-event > > Yes it is true that this event was used by Orca. In fact, it is still used, > as Joanie is modifying Orca in order to only take care of > atkobject:state-changed:focused. And it is true that if firefox stop to emit > AtkObject::focus-event, and the user is using an old version of Orca, it > would not receive those a event is listening too. But we didn't see that as > a big problem. Right now, it is really usual that each Firefox update needs > a Orca tweak. Probably you are already aware due this bug [4]. Another > example here [3]. So a recent Firefox needs a recent Orca. sure, we certainly have bugs that cause newer orca's to work better, but I don't think we should go around creating more reason you need a newer orca unless we have to, and I don't see one of those here. So I think we should back this patch out and add a comment saying // XXX kept for compatibility with old AT. > In any case, probably a compromise would be keep emitting > AtkObject::focus-event if you are worried about old versions of Orca. Newer > versions of Orca will just ignore it. yeah, I don't see any harm in backing this out until atk changes abi.

(In reply to Trevor Saunders (:tbsaunde) from comment #9) > but I > don't think we should go around creating more reason you need a newer orca > unless we have to, and I don't see one of those here. So I think we should > back this patch out and add a comment saying // XXX kept for compatibility > with old AT. It's usual practice when Firefox requires up-to-date AT, we did that number of times with Orca and NVDA. We do an exception and we keep backward compatibility for proprietary screen readers only. As long as updated Orca will be released when Firefox 28 is out then I wouldn't bother about compatibility here.

(In reply to alexander :surkov from comment #10) > (In reply to Trevor Saunders (:tbsaunde) from comment #9) > > but I > > don't think we should go around creating more reason you need a newer orca > > unless we have to, and I don't see one of those here. So I think we should > > back this patch out and add a comment saying // XXX kept for compatibility > > with old AT. > > It's usual practice when Firefox requires up-to-date AT, we did that number > of times with Orca and NVDA. We do an exception and we keep backward > compatibility for proprietary screen readers only. As long as updated Orca > will be released when Firefox 28 is out then I wouldn't bother about > compatibility here. I'd disagree with that we should support people using the orca that comes on the oldest distro we support people running firefox on. especially when there's no reason to break things.

Comment on attachment 828089 [details] [diff] [review] patch adjusting review since I made wrong assumptions about usage of focus event.

(In reply to Trevor Saunders (:tbsaunde) from comment #12) > Comment on attachment 828089 [details] [diff] [review] > patch > > adjusting review since I made wrong assumptions about usage of focus event. what your point is?

(In reply to alexander :surkov from comment #13) > (In reply to Trevor Saunders (:tbsaunde) from comment #12) > > Comment on attachment 828089 [details] [diff] [review] > > patch > > > > adjusting review since I made wrong assumptions about usage of focus event. > > what your point is? same as comment 9 we should back this out because it breaks stuff and doesn't help anything.

(In reply to Trevor Saunders (:tbsaunde) from comment #11) > I'd disagree with that we should support people using the orca that comes > on the oldest distro we support people running firefox on. especially when > there's no reason to break things. If people want compatibility with old distros then they use ESR release so supposedly they have enough time to switch before our changes hit them.

(In reply to alexander :surkov from comment #15) > (In reply to Trevor Saunders (:tbsaunde) from comment #11) > > > I'd disagree with that we should support people using the orca that comes > > on the oldest distro we support people running firefox on. especially when > > there's no reason to break things. > > If people want compatibility with old distros then they use ESR release so > supposedly they have enough time to switch before our changes hit them. I don't believe its true those people only run esr. In any case if it will run in some enviroment then I think we should generally try and have it be accessible there. Put another way is there any good reason we _should_ make this change? because I don't think there is.

(In reply to Trevor Saunders (:tbsaunde) from comment #16) > Put another way is there any good reason we _should_ make this change? > because I don't think there is. yes, it makes sense. On the other hand if Orca developers think that's ok to drop old Orca's support then I don't really have a good reason to insist on opposite thing.

(In reply to alexander :surkov from comment #17) > (In reply to Trevor Saunders (:tbsaunde) from comment #16) > > > Put another way is there any good reason we _should_ make this change? > > because I don't think there is. > > yes, it makes sense. On the other hand if Orca developers think that's ok to > drop old Orca's support then I don't really have a good reason to insist on > opposite thing. I don't think they've said that, and the last bit of comment 4 actually sort of suggests keeping it.

Ok, I don't mind on either way.

Do you think we should reopen bug?

(In reply to alexander :surkov from comment #20) > Do you think we should reopen bug? I think i've already said I think we should ;)

before that I'd be good to have Joanie or API for their thoughts again since API said in comment #5: "stop to use all those methods are safe from the POV of any version of an AT."

(In reply to alexander :surkov from comment #22) > before that I'd be good to have Joanie or API for their thoughts again since > API said in comment #5: > "stop to use all those methods are safe from the POV of any version of an > AT." which is the part we're actually talking about... If you read the whole comment you'd also see > So the other element deprecated: > AtkObject::focus-event > > Yes it is true that this event was used by Orca. In fact, it is still used, > as Joanie is modifying Orca in order to only take care of > atkobject:state-changed:focused. And it is true that if firefox stop to emit > AtkObject::focus-event, and the user is using an old version of Orca, it > would not receive those a event is listening too. But we didn't see that as > a big problem. Right now, it is really usual that each Firefox update needs > a Orca tweak. Probably you are already aware due this bug [4]. Another > example here [3]. So a recent Firefox needs a recent Orca. > > In any case, probably a compromise would be keep emitting > AtkObject::focus-event if you are worried about old versions of Orca. Newer > versions of Orca will just ignore it.

(In reply to Trevor Saunders (:tbsaunde) from comment #23) > (In reply to alexander :surkov from comment #22) > > before that I'd be good to have Joanie or API for their thoughts again since > > API said in comment #5: > > "stop to use all those methods are safe from the POV of any version of an > > AT." > > which is the part we're actually talking about... > If you read the whole comment you'd also see ok, I see. If you want to backout the patch then it's fine with me. And then let's keep the bug open until it's absolutely safe to fix it.