After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 657646 - Default Speech stopping on active descendant change is too restrictive
Default Speech stopping on active descendant change is too restrictive
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Orca Maintainers
Orca Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-08-29 17:46 UTC by Alejandro Piñeiro Iglesias (IRC: infapi00)
Modified: 2011-08-29 23:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fixes the bug (1.01 KB, patch)
2011-08-29 17:48 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
reviewed Details | Review
Updated patch (1.06 KB, patch)
2011-08-29 18:09 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
none Details | Review
Updated patch (2) (1.06 KB, patch)
2011-08-29 18:18 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
none Details | Review
Updated (3) (1.03 KB, patch)
2011-08-29 18:22 UTC, Alejandro Piñeiro Iglesias (IRC: infapi00)
committed Details | Review

Description Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-29 17:46:23 UTC
At this moment, by default any active-descendant-change event will means a speech stop.

But IMHO, this is too restrictive as the following environment could be really common:

 * A container and their children
   * This container is the one that will have the state STATE_MANAGES_DESCENDANTS
 * Container receives the focus event => focus speech
 * First selection/active item => active-descendant event => active item speech

But with the default behaviour the active event will stop the speech of the focused object. And that could be in general misleading, as the container provides the context of the situation.
Comment 1 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-29 17:48:03 UTC
Created attachment 195104 [details] [review]
Fixes the bug
Comment 2 Joanmarie Diggs (IRC: joanie) 2011-08-29 17:56:44 UTC
Review of attachment 195104 [details] [review]:

-        return True
+        if (event.source == orca_state.locusOfFocus) \
+                and (event.any_data.parent == event.source):
+            return False
+        else:
+            return True

One nit: this is not C or C++ (The parens are unneeded and while they don't hurt anything, they annoy me. :-P Mind removing them?)

One actual potential problem: We have no guarante that we'll have event.any_data. Mind you, we should, but.... If event.any_data == None, your event.any_data.parent check is going to traceback and that will kill speech and possibly the further processing of the new event. So... if there's not event.any_data, let's return True (more conservative/less change). Make sense?

Thanks!
Comment 3 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-29 18:09:16 UTC
Created attachment 195107 [details] [review]
Updated patch
Comment 4 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-29 18:18:23 UTC
Created attachment 195108 [details] [review]
Updated patch (2)

With some extra suggestions.

Anyway, I still need to further test it.
Comment 5 Alejandro Piñeiro Iglesias (IRC: infapi00) 2011-08-29 18:22:54 UTC
Created attachment 195111 [details] [review]
Updated (3)
Comment 7 Joanmarie Diggs (IRC: joanie) 2011-08-29 23:00:14 UTC
I think the change is sound and good and was worth including in 3.1.90.

I'm going to close this bug out. Seeing as how Orca from master is now the text-book example of unstable, I'll be opening a new 3.1.90 is borked meta-bug later today or tomorrow. ;-) New issues can go on it, should they exist w.r.t. this bug.