GNOME Bugzilla – Bug 448848
Port Orca to pyatspi
Last modified: 2008-07-22 19:32:37 UTC
Pyatspi is the official AT-SPI python wrapper. It should replace all the wrappers that ATs have been writing on their own.
I should have a modified version of atspi.py here soon that simply wraps pyatspi and gives API compatibility, so we could migrate over slowly.
Here are some issues I already came across that will need some decision making. • When a callback raises an exception in pyatspi the exception is handled while the stack trace is put in stderr. This is similar to Orca's behavior, but does not use Orca's debug facility. • in atspi.Accessible.__get_role() We have a few hacks to fix some toolkit anomalies. Is this the place? If so, this needs to go upstream to pyatspi, since ultimately we will do away with atspi.py all together.
Created attachment 93412 [details] [review] Turn atspi.py into a compatability wrapper This patch butchers pyatspi into a proxy module. It also adds a deprecation warning debug level.
(In reply to comment #3) > Created an attachment (id=93412) [edit] > Turn atspi.py into a compatability wrapper > > This patch butchers pyatspi into a proxy module. > It also adds a deprecation warning debug level. Cool! This is a pretty involved change, but looks like it is headed in the right direction. I'm curious about a few things, mostly due to laziness an not looking at the pyatspi implementation. 1) There's a settings.cacheValues field that you might use to set pyatspi.setCacheLevel(pyatspi.CACHE_PROPERTIES), I think. I'm not sure exactly what CACHE_PROPERTIES does, though. 2) The old event registration code attempts to register only one event listener for any given type with the atspi registry no matter how many times one registers an event. Does pyatspi 3) Rather than introducing a new debug level for deprecation, I think WARNING is probably sufficient. It basically means we have a potential problem, but Orca can deal with it without crashing. (In reply to comment #2) > Here are some issues I already came across that will need some decision making. > > • When a callback raises an exception in pyatspi the exception is handled > while the stack trace is put in stderr. This is similar to Orca's behavior, but > does not use Orca's debug facility. One reason we avoided stderr was to prevent flooding a gnome-terminal with events when Orca was run from a terminal -- this could cause a circle of event violence with Orca. By being able to redirect debug to file, we got around this. I suppose we could always redirect stderr to the same file. Where in pyatspi.py does it handle these kinds of things. Would it make sense to try to rethrow the exception so Orca can be aware of them, or are these just benign things that pyatspi can gracefully handle? > • in atspi.Accessible.__get_role() We have a few hacks to fix some toolkit > anomalies. Is this the place? If so, this needs to go upstream to pyatspi, > since ultimately we will do away with atspi.py all together. They are hacks in Orca to account for hacks elsewhere in the infrastructure. :-( It would be nice to be able to make them go upstream to pyatspi. Some probably need to go upstream into Orca, however, since a couple of the rolenames are synthetic (CHECK_MENU and RADIO_MENU) and aren't known anywhere outside Orca. This is going to be good stuff. Pyatspi is going to need a lot of work with respect to causing memory leaks in applications (e.g., it needs to unref things appropriately), but everyone will win overall with this, I think.
> 1) There's a settings.cacheValues field that you might use to set > pyatspi.setCacheLevel(pyatspi.CACHE_PROPERTIES), I think. I'm not sure exactly > what CACHE_PROPERTIES does, though. > I added that here in my local copy, I'll post an updated patch soon. > 2) The old event registration code attempts to register only one event listener > for any given type with the atspi registry no matter how many times one > registers an event. Does pyatspi > Yes, registry.py:662 > 3) Rather than introducing a new debug level for deprecation, I think WARNING > is probably sufficient. It basically means we have a potential problem, but > Orca can deal with it without crashing. > Right now we are spewing out a very large amount of deprecated warnings, I think it makes it hard to watch other debug info. Not sure what to do about it really, since the debug system does not allow masking. > > > > • When a callback raises an exception in pyatspi the exception is handled > > while the stack trace is put in stderr. This is similar to Orca's behavior, but > > does not use Orca's debug facility. > > One reason we avoided stderr was to prevent flooding a gnome-terminal with > events when Orca was run from a terminal -- this could cause a circle of event > violence with Orca. By being able to redirect debug to file, we got around > this. I suppose we could always redirect stderr to the same file. > > Where in pyatspi.py does it handle these kinds of things. Would it make sense > to try to rethrow the exception so Orca can be aware of them, or are these just > benign things that pyatspi can gracefully handle? > I already ran into this when I was working with Accerciser. pyatspi used to silently handle _all_ exceptions that occurred in a callback. In a screen reader, that's about 95% of the code. Peter put in a stack trace printout, this is similar to Orca's current implementation too. It's kind of like pygtk I guess, since it will also handle all exceptions thrown in a callback, but print them out to stderr. > > • in atspi.Accessible.__get_role() We have a few hacks to fix some toolkit > > anomalies. Is this the place? If so, this needs to go upstream to pyatspi, > > since ultimately we will do away with atspi.py all together. > > They are hacks in Orca to account for hacks elsewhere in the infrastructure. > :-( It would be nice to be able to make them go upstream to pyatspi. Some > probably need to go upstream into Orca, however, since a couple of the > rolenames are synthetic (CHECK_MENU and RADIO_MENU) and aren't known anywhere > outside Orca. Yeah, if we invented new role types, they shouldn't go up to pyatspi ;) Would it be a painful and error-prone process to put these in at a later stage of the code path, say the toolkit scripts? they are after all compensating for bugs in bridges (like the Java hack). Ultimately, they should go up the other stream (ie. the actual bridges, and the at-spi IDL) > > This is going to be good stuff. Pyatspi is going to need a lot of work with > respect to causing memory leaks in applications (e.g., it needs to unref things > appropriately), but everyone will win overall with this, I think. > Yeah, I started thinking about all that. Maybe we should work out a test plan for memory leaks? Maybe record a 30 minute session of random activity and do gc dumps to a file once in a while. All in all, memory leaks aren't really a critical blocker for me, Orca will be a great tool to flush out the leaks from pyatspi as we go. I would also love to compare performance, and make sure that we only improve responsiveness and don't regress on that front. How would you go about measuring that?
(In reply to comment #5) > > 1) There's a settings.cacheValues field that you might use to set > > pyatspi.setCacheLevel(pyatspi.CACHE_PROPERTIES), I think. I'm not sure exactly > > what CACHE_PROPERTIES does, though. > > > I added that here in my local copy, I'll post an updated patch soon. Cool. Thanks! > > 2) The old event registration code attempts to register only one event listener > > for any given type with the atspi registry no matter how many times one > > registers an event. Does pyatspi > > > > Yes, registry.py:662 Excellent. Thanks! > > 3) Rather than introducing a new debug level for deprecation, I think WARNING > > is probably sufficient. It basically means we have a potential problem, but > > Orca can deal with it without crashing. > > > > Right now we are spewing out a very large amount of deprecated warnings, I > think it makes it hard to watch other debug info. Not sure what to do about it > really, since the debug system does not allow masking. It really depends upon how you want to treat the deprecation messages. I'd view them as warnings that should be output at the WARNING level. We definitely want them brought to our (or your ;-)) attention to make sure things are flushed out. But, if that proves to be way too much information for the short term, I guess it does make sense to make a new debug.LEVEL_DEPRECATION constant. Maybe make it -1 for the short term if you want it to be very very silent, but then up it to WARNING once you're migration to pyatspi nears completion. > > > • When a callback raises an exception in pyatspi the exception is handled > > > while the stack trace is put in stderr. This is similar to Orca's behavior, but > > > does not use Orca's debug facility. > > > > One reason we avoided stderr was to prevent flooding a gnome-terminal with > > events when Orca was run from a terminal -- this could cause a circle of event > > violence with Orca. By being able to redirect debug to file, we got around > > this. I suppose we could always redirect stderr to the same file. > > > > Where in pyatspi.py does it handle these kinds of things. Would it make sense > > to try to rethrow the exception so Orca can be aware of them, or are these just > > benign things that pyatspi can gracefully handle? > > > > I already ran into this when I was working with Accerciser. pyatspi used to > silently handle _all_ exceptions that occurred in a callback. In a screen > reader, that's about 95% of the code. Peter put in a stack trace printout, this > is similar to Orca's current implementation too. It's kind of like pygtk I > guess, since it will also handle all exceptions thrown in a callback, but print > them out to stderr. If I understand this correctly, we should definitely trying to be handling exceptions in Orca code. So, dumping them to stderr seems like the right thing to do -- we want unhandled exceptions brought to our attention. If need be, I think we can always redefine sys.stderr to be the debug output file, right? > > > • in atspi.Accessible.__get_role() We have a few hacks to fix some toolkit > > > anomalies. Is this the place? If so, this needs to go upstream to pyatspi, > > > since ultimately we will do away with atspi.py all together. > > > > They are hacks in Orca to account for hacks elsewhere in the infrastructure. > > :-( It would be nice to be able to make them go upstream to pyatspi. Some > > probably need to go upstream into Orca, however, since a couple of the > > rolenames are synthetic (CHECK_MENU and RADIO_MENU) and aren't known anywhere > > outside Orca. > > Yeah, if we invented new role types, they shouldn't go up to pyatspi ;) > Would it be a painful and error-prone process to put these in at a later stage > of the code path, say the toolkit scripts? they are after all compensating for > bugs in bridges (like the Java hack). It would be painful for anything that depends upon CHECK_MENU and RADIO_MENU being an acceptable value for a role. This includes the speech/braille generators. But...to think of the real world use case for these synthetic roles: there aren't really any that I know of. They were made up to accommodate gtk-demo's odd menu demo. I think it may be safe to just drop them. > > This is going to be good stuff. Pyatspi is going to need a lot of work with > > respect to causing memory leaks in applications (e.g., it needs to unref things > > appropriately), but everyone will win overall with this, I think. > > > > Yeah, I started thinking about all that. Maybe we should work out a test plan > for memory leaks? Maybe record a 30 minute session of random activity and do gc > dumps to a file once in a while. All in all, memory leaks aren't really a > critical blocker for me, Orca will be a great tool to flush out the leaks from > pyatspi as we go. Just to be sure: it's not memory leaks in the Orca process, it's memory leaks in the application process. So, we need to keep track of what's not being unref'd when it should be. > I would also love to compare performance, and make sure that we only improve > responsiveness and don't regress on that front. How would you go about > measuring that? I'd like to see that, too. I guess the python profiler might help with this: http://docs.python.org/lib/profile-instant.html It might be something to try to hook into (or do as an alternative) to the code coverage option of the test harness. That is -- instead of (or in addition to) measuring code coverage of the overall test suite, we measure performance.
Changed patch to committed. We now have a pyatspi branch.
Pychecker found this problem in braillegenerator.py on trunk: /usr/lib/python2.4/site-packages/orca/braillegenerator.py:1201: Variable (i) used before being set /usr/lib/python2.4/site-packages/orca/braillegenerator.py:1586: No global (i) found The code is this in trunk: for child in obj: try: action = child.queryAction() except: continue else: for j in range(0, action.nActions): if action.getName(j) == "toggle": hasToggle[i] = True break It is this in the gnome-2-20 branch: for i in range(0, obj.childCount): action = obj.child(i).action if action: for j in range(0, action.nActions): if action.getName(j) == "toggle": hasToggle[i] = True break This looks like fallout from pyatspi integration.
Another similar pychecker problem in the same file: /usr/lib/python2.5/site-packages/orca/braillegenerator.py:1587: No global (i) found for child in obj: debug.println(debug.LEVEL_FINEST, "braillegenerator.getBrailleRegions " \ + "looking at child %d" % i)
Created attachment 97310 [details] [review] add getInterface() and hasState() to default.py Will/Eitan?
hasState doesn't seem like it gets us any convenience win by living in default.py: obj.getState().contains(state) vs. self.hasState(obj, state) getInterface seems useful, though.
Created attachment 97391 [details] [review] revised getInterface()
After more discussion in #orca, the decision was made to not have a getInterface() after all, but to create a queryNonEmptyText(obj) for Gecko.py. It'll go into the next Gecko.py check in I do.
Added additional exception handling (AttributeError) to braillegenerator.getBrailleContext(). This was necessary because of an exception I saw in gmail. Although gmail is known to notoriously problematic, this is a simple fix that does not impact performance or change the logical flow for 'correct' accessible hierarchies. It simply is a defensive maneuver for this problematic site. committed to trunk.
fixed string split issue at line 116 in gnome-mud.py. committed to trunk.
wrapped queryAction() in try/except in flat_review._insertStateZone().
pyatspi migration related, changed getValue() to queryValue() in default.handleProgressBarUpdate(). committed to trunk.
Eitan - feel free to close this bug now. Thanks for the hard work!