GNOME Bugzilla – Bug 634642
Orca can be very slow to find position in a Gecko list
Last modified: 2011-09-04 06:26:37 UTC
In a large list orca may be slow to find the position. As an example a large email folder in thunderbird (eg. in my case about 500 messages) it can take orca about 6 seconds to find the position.
In the thunderbird case I have tracked the slow performance to the following code in speech_generator.py in the generatePositionInList method: if not total: for child in obj: nextName = self._generateName(child) state = child.getState() if not nextName or nextName[0] in ["", "Empty", "separator"] \ or not state.contains(pyatspi.STATE_VISIBLE): continue index += 1 total += 1 if nextName == name: position = index Looking at this most of it seems to be pyatspi and so may be at-spi stuff. The only call to other orca code is the self._generateName(child) call. Question for those who know more about at-spi, might it be best to try this with the dbus at-spi to seem how things perform there (IE. As the dbus version of at-spi is the future and if the issue is in at-spi then using at-spi on corba may not really be of value in solving this bug)?
Adding myself to to this bug.
I elieve this is blocked by https://bugzilla.mozilla.org/show_bug.cgi?id=527461
O, on looking at this more I don't think that mozilla bug blocks this. However if gecko does expose the attributs discussed position in group etc then we might be able to make doing this in gecko very fast by not figuring it out ourself. As for optimizing this the fact we need to check the name makes things unfortunite, I think the best thing we coulddothere isto first continue immediately if we find the pyatspi state not to be what we want since that is only 1 round trip at most where as generateName() I believe could be many.
Created attachment 178932 [details] [review] Ugly patch that makes this really fast in gecko apps using the posinset and setsize attributes gecko gives on some objects.
(In reply to comment #5) > Created an attachment (id=178932) [details] [review] > Ugly patch that makes this really fast in gecko apps using the posinset and > setsize attributes gecko gives on some objects. It may be ugly, but produced a miracle. Turned water into wine. I tested in my trash folder that contains 13784 messages and the result was excelent. needs to be applied when using the flat mode. Great job.
Trevor, This is wonderful! I looking now 3.1.7 version of Thunderbird with turned on position list spokening feature again, very fast the message spokening. My inbox have 1587 messages, when I jumping in box tree and press Tab key, I hear after Tab key press the caret moved message informations, and the position index is full right (for example 1586 of 1587). Only my problem Thunderbird 3.1.7 is not working right the message presentation with a new window, but this is another story and happen prewious mail changes in Orca-list why not possible putting a patch this related with Orca side.
Jose, what do you mean needs to be applied when using flat mode? A quick perf note with the patch applied I still see orca using ~34 seconds (according to ps) to move up and down through a mailbox of ~300 messages. I did that in a about five minutes without actually reading anything, so this isn't bad but could be significantly improved.
Created attachment 178947 [details] Debug.out with show a traceback Trewor, I see following traceback error message when I moving only folders in Thunderbird 3.0.10, I think important: Traceback (most recent call last):
+ Trace 225634
globalsDict[arg] = self._methodsDict[arg](obj, **args)
if nextName == name:
I looking this because some time not spokened Orca the mail folder name when I jumping back the folder list with Shift+Tab key. Only extended folders happening this (subfolders). Possible fix this?
(In reply to comment #8) > Jose, what do you mean needs to be applied when using flat mode? > When I am in the list of messages and try moves the flat review to the end position, orca takes some time to respond.
I haven't had a chance to apply the patch yet but other recent fixes made the message list navigate quickly. I usually haven't cared much about the number of items in the list because I use a threaded view but it is valuable information, none-the-less. I agree with other comments about flat review. flat review is practically unusable in the message list! That is no fault of any of these recent improvements to Orca but just goes to show how badly flat review needs help in general.
I haven't tried the patch. Here are some thoughts regarding it though. * If as suggested in the mozilla report this is something only done by gecko then it probably needs to be constrained to gecko scripts or may be thunderbird/other gecko applications where it applies. * I seem to remember (I haven't checked in the orca code since I reported this) the code I identified as slow is default code and so may apply outside gecko. Therefore the problem could show itself in other applications should orca need to fall back on this checking every element of a list/tree. NOTE: In the second case, I have no actual example and I am just thinking it is a potential issue. So yes if the patch works, good it helps in the specific case of thunderbird but we may still need to find a more generic fix as well. (In reply to comment #5) > Created an attachment (id=178932) [details] [review] > Ugly patch that makes this really fast in gecko apps using the posinset and > setsize attributes gecko gives on some objects.
Attila, is that a regression? Or did it exist before? It seems to me like its very like that's some sort of existing issue.
I just applied the patch and I see two big improvements. The speedup is incredible. Also, I noticed the message count in the list was completely wrong with the previous versions of Orca but once patched, the item count is now being correctly reported. I had forgotten about the incorrect item counts in Thunderbird lists before now. I really hope this patch or something verry similar will be applied soon to master. Thanks Trever for the fix.
Trev, I just took an *extremely* quick look. It sounds like this is working and doing the correct thing. You, sir, are awesome! From my brief glance I'm guessing that a lot of code got duplicated from the original (non-Gecko subclassed) speech generator. If so, would it instead make more sense to try to get the desired attributes and, failing that, return whatever the parent class would return?
P.S. Trev, you should have edit bugs privs as our triager. Therefore, next time you're in this bug, please assign it to yourself, and set the Target Milestone (which is *not* the GNOME target) to 2.91.6. Thanks!
(In reply to comment #16) > P.S. Trev, you should have edit bugs privs as our triager. Therefore, next time > you're in this bug, please assign it to yourself, and set the Target Milestone > (which is *not* the GNOME target) to 2.91.6. Thanks! I thought about that before, but I wasn't eager to asign it to myself since I was hoping someone who knew more would rework my proof of concept into a nice clean patch.
Sure, I'll trade you that for whatever mystery issues remain regarding the event and script management refactor. <grins> All kidding aside, look through the Gecko speech generator -- or for that matter any of the other subclassed speech generators (in fact Gecko being Gecko, I'd encourage one of the others for simplicity's sake). Anyhoo.... What you should find is case after case of: def blah(self, obj, **args): if uniqueSituationIsFound(): result = foobar() else: result = speech_generator.SpeechGenerator.blah(self, obj, **args) If all you have done differently is see if those Gecko-specific obj attributes are present, there's your unique situation. Make sense?
(In reply to comment #16) > P.S. Trev, you should have edit bugs privs as our triager. Therefore, next time > you're in this bug, please assign it to yourself, and set the Target Milestone > (which is *not* the GNOME target) to 2.91.6. Thanks! I thought about that before, but I wasn't eager to asign it to myself since I was hoping someone who knew more would rework my proof of concept into a nice clean patch. (In reply to comment #18) > Sure, I'll trade you that for whatever mystery issues remain regarding the > event and script management refactor. <grins> heh, I wanted someone else to do it so it wouldn't need to be refactored later :-) > All kidding aside, look through the Gecko speech generator -- or for that > matter any of the other subclassed speech generators (in fact Gecko being > Gecko, I'd encourage one of the others for simplicity's sake). Anyhoo.... What > you should find is case after case of: > > def blah(self, obj, **args): > if uniqueSituationIsFound(): > result = foobar() > else: > result = speech_generator.SpeechGenerator.blah(self, obj, **args) Yeah I could do that, I just wasn't sure if that might cause incorrect results in other places where the other bits of the default function would work where this wouldn't although that shouldn't be the case. > If all you have done differently is see if those Gecko-specific obj attributes > are present, there's your unique situation. > > Make sense? Yeha, I'll implement and post that patch to see if people run into any issues with it.
(In reply to comment #16) > P.S. Trev, you should have edit bugs privs as our triager. Therefore, next time > you're in this bug, please assign it to yourself, and set the Target Milestone > (which is *not* the GNOME target) to 2.91.6. Thanks! I thought about that before, but I wasn't eager to asign it to myself since I was hoping someone who knew more would rework my proof of concept into a nice clean patch. (In reply to comment #18) > Sure, I'll trade you that for whatever mystery issues remain regarding the > event and script management refactor. <grins> heh, I wanted someone else to do it so it wouldn't need to be refactored later :-) > All kidding aside, look through the Gecko speech generator -- or for that > matter any of the other subclassed speech generators (in fact Gecko being > Gecko, I'd encourage one of the others for simplicity's sake). Anyhoo.... What > you should find is case after case of: > > def blah(self, obj, **args): > if uniqueSituationIsFound(): > result = foobar() > else: > result = speech_generator.SpeechGenerator.blah(self, obj, **args) Yeah I could do that, I just wasn't sure if that might cause incorrect results in other places where the other bits of the default function would work where this wouldn't although that shouldn't be the case. > If all you have done differently is see if those Gecko-specific obj attributes > are present, there's your unique situation. > > Make sense? Yeha, I'll implement and post that patch to see if people run into any issues with it. (In reply to comment #18) > Sure, I'll trade you that for whatever mystery issues remain regarding the > event and script management refactor. <grins> > > All kidding aside, look through the Gecko speech generator -- or for that > matter any of the other subclassed speech generators (in fact Gecko being > Gecko, I'd encourage one of the others for simplicity's sake). Anyhoo.... What > you should find is case after case of: > > def blah(self, obj, **args): > if uniqueSituationIsFound(): > result = foobar() > else: > result = speech_generator.SpeechGenerator.blah(self, obj, **args) > > If all you have done differently is see if those Gecko-specific obj attributes > are present, there's your unique situation. > > Make sense? p.s. I also added a function getAttribute(self, obj, "attributename") that does what you think, and imho that really shouldn't live in the gecko script, but I'm not really sure where to put it.
Trevor, prewious I think I not see this problem before I not apply the patch in Thunderbird 3.0.10, but just a moment I looking an unpached compiled orca this problem and tell the results. Attila
Trevor, I verifyed I reported problem. I pulled latest Orca source and doed a git reset --hard command with master branch. After this I full recompiled and reinstall Orca. When I navigating folders list in Thunderbird, I hear all folder names with Up or Down arrow keys in folder list, I hear subfolders too, or I press Shift+Tab key, I hear all right the folder name in Thunderbird 3.0.10. When I using the patch with fastest list position founding, the problem is come back, I not hear subfolders. Attila
Trevor, an important information, Thunderbird 3.1.7 not producing this problem, not have any traceback error message in debug.out file if I using Thunderbird 3.1.7 version with patch testing purpose. Attila
Created attachment 179035 [details] [review] cleaned up patch. here's a cleaner patch fixing this issue, getAttribute() still needs to live somewhere else, but that's an easy fix. There is also apossible fix for this issue in Gecko there are try server builds for it at x86 64 http://stage.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/trev.saunders@gmail.com-8d05d4ab5531/tryserver-linux64/thunderbird-3.3a3pre.en-US.linux-x86_64.tar.bz2 and for x86 http://stage.mozilla.org/pub/mozilla.org/thunderbird/tryserver-builds/trev.saunders@gmail.com-8d05d4ab5531/tryserver-linux/thunderbird-3.3a3pre.en-US.linux-i686.tar.bz2 I'd like to here what people think of the try server build. I think from the code the orca patch should be faster, but I'm not aware of any reasonable way to profile this so I have no evidence I am correct:-)
Trevor, thank you the fix, works very good in Thunderbird 3.0.10 and Thunderbird 3.1.7. Attila
(In reply to comment #25) Attila, would you mind comparing the two different fixes in comment 24?
Trevor, I tryed your linked Thunderbird version (the serwer build) without the Orca patch. The problem unfortunately is present, the list operations is very slow with large mail folders, and the spokened position index values is invalid. If this is only the testing purpose, the Orca patch is need to solving this problem with Orca level. Or, if I doed wrong test, tell me what need testing this version. I applying your patch again, and looking how works this linked Thunderbird version. Attila
Trevor, I applied again your patch and try using the you linked Thunderbird version. List navigation is very fast, Orca position index spokening feature spokening right index values, when I navigating the mail subfolders, I hear right all folder names. I don't see any errors with your patch related.
(In reply to comment #27) > Trevor, I tryed your linked Thunderbird version (the serwer build) without the > Orca patch. > The problem unfortunately is present, the list operations is very slow with > large mail folders, and the spokened position index values is invalid. If this > is only the testing purpose, the Orca patch is need to solving this problem > with Orca level. > Or, if I doed wrong test, tell me what need testing this version. I applying > your patch again, and looking how works this linked Thunderbird version. > > Attila (In reply to comment #27) > Trevor, I tryed your linked Thunderbird version (the serwer build) without the > Orca patch. Ok, thanks! > The problem unfortunately is present, the list operations is very slow with > large mail folders, and the spokened position index values is invalid. If this > is only the testing purpose, the Orca patch is need to solving this problem > with Orca level. There was a patch applied implementing role_node_parent_of applied for that build, but it looks like there's a bug in it or something. So, It looks like it will take more time to get role_node_parent_of working to fix this problem. I also suspect that the current solution mozilla gives is better than role_node_parent_of particularly I believe that we'll need O(n) time to find the position even with role_node_parent_of but mozilla already gives it to us in O(1). So, since I think the mozilla guys have better things to be doing right now than implementing fixes that are probably worse than existing fixes I'm poking joanie for review of the existing patch, and hoping she'll be willing to move getAttribute to a more senseible place oh, btw Attila, does the stack trace still happen with the new patch, I assume the root cause is a bug that was fixed since thunderbird 3.0
Trevor, no, not have any errors when I appliing your last attached Orca patch. Your last attached Orca patch fixed right I prewious wrote traceback error message. This error happening only Thunderbird 3.0.10 when I appliing your first patch. Without the first attached patch Thunderbird 3.0.10 don't producing this issue with I related wrote traceback error message with a prewious comment, but this is resolved right your last patch. Attila
Created attachment 179168 [details] Debug file with show an another problem Trevor, I found an important bug after applied the cleanup second patch (your last patch). With Firefox and Thunderbird, the submenues spokened position index values is not alwais right. Reproducation steps to verify: 1. First, if not checked, check the speak child position check box. 2. Launch firefox, and listen for example the file main menu menu items position index values. The spokened position index values is not alwais right my machine. Important, need listening all position index values for example with file menu when testing. Another reproducation example: 1. Launch Thunderbird (I tested Thunderbird 3.1.7, Thunderbird 3.3A3Pre. 2. Pull down for example the file main menu, and listen the position index values. The spokened position index values is not alwais will be right. When I not using any this bugreport related patch, the spokened position index values is full right with a main menu menu items, but not right with message list for example in Thunderbird. Anybody confirming this test? Attila Attila items
Created attachment 179275 [details] [review] Temporari remove the position index value for Gecko formatting Hy, Trevor, I doed with I wrote in Orca list (the last way fix). :-):-) Temporari I removed now not correct position index values from src/orca/scripts/toolkits/Gecko/formatting.py file. I inserted all menu related objects from src/orca/formatting.py file with Gecko formatting.py file, and removed the position index spokening with unfocused object only. Please test if not have another solution to fix this problem. Now I not removed where am I and detailed where am I the positionindex spokening (where am I and detailed where am I spokening too this new incorrect position index value). I don't no real incorrect or not this value, because perhaps this value mean a menu group with have x menu items. Attila
diff --git a/src/orca/scripts/toolkits/Gecko/formatting.py b/src/orca/scripts/toolkits/Gecko/formatting.py index 5d2adfd..ac7898c 100644 --- a/src/orca/scripts/toolkits/Gecko/formatting.py +++ b/src/orca/scripts/toolkits/Gecko/formatting.py @@ -41,6 +41,7 @@ import orca.settings # getFormat method). # # # ######################################################################## + formatting = { nit, but this is unrelated. 'speech': { 'suffix': { @@ -77,6 +78,33 @@ formatting = { pyatspi.ROLE_TABLE: { 'unfocused': '[]' }, + pyatspi.ROLE_CHECK_MENU_ITEM: { + 'focused': 'checkedState', + 'unfocused': 'labelAndName + roleName + checkedState + required + availability + ' + orca.formatting.MNEMONIC + ' + accelerator', + 'basicWhereAmI': 'ancestors + labelAndName + roleName + checkedState + accelerator + positionInList + ' + orca.formatting.MNEMONIC + }, + pyatspi.ROLE_MENU: { + 'focused': '[]', + 'unfocused': 'labelAndName + allTextSelection + roleName + availability + ' + orca.formatting.MNEMONIC + ' + accelerator', + 'basicWhereAmI': '(ancestors or parentRoleName) + labelAndName + roleName + positionInList + ' + orca.formatting.MNEMONIC + }, + pyatspi.ROLE_MENU_ITEM: { + 'focused': '[]', + 'unfocused': 'labelAndName + menuItemCheckedState + availability + ' + orca.formatting.MNEMONIC + ' + accelerator', + 'basicWhereAmI': 'ancestors + labelAndName + accelerator + positionInList + ' + orca.formatting.MNEMONIC + }, I'm really not sure how this data structure works, but at first glance it seems like this might do what you say it does. + pyatspi.ROLE_RADIO_MENU_ITEM: { + # OpenOffice check menu items currently have a role of "menu item" + # rather then "check menu item", so we need to test if one of the + # states is CHECKED. If it is, then add that in to the list of + # speech utterances. Note that we can't tell if this is a "check + # menu item" that is currently unchecked and speak that state. + # See Orca bug #433398 for more details. + # Why are you including something openoffice specific into gecko code? Please don't do this unless the code is actually needed for Gecko too in which case please update the comment. + 'focused': 'labelAndName + radioState + roleName + availability + positionInList', + 'unfocused': 'labelAndName + radioState + roleName + availability + ' + orca.formatting.MNEMONIC + ' + accelerator', + 'basicWhereAmI': 'ancestors + labelAndName + roleName + radioState + accelerator + positionInList + ' + orca.formatting.MNEMONIC + }, }, 'braille': { # [[[TODO: WDW - we're doing very little here. The goal for
Trevor, I added radio menu item related formatting values only vecause I don't no Gecko using this role type or not. Sorry, I forgot remove the comment. Just a moment, I doing. Attila
Created attachment 179283 [details] [review] Temporary removed the positioninlist with Gecko formatting the menu objects related Cleaned patch with not containing Trevor founded unneed empty line and Openoffice.org related comment with pyatspi.role_Radio_Menu_Item object. Removed incorrect positionin list index value spokening with menu and menu related objects (menu items, submenues, check menu items etc) with src/orca/scripts/toolkits/Gecko/formatting.py file. This fix need choosing only if not possible restore the original position index values spokening with only this menu related objects. Attila
Created attachment 179767 [details] [review] patch restoring the old way of speaking position in a list. Same patch as before, but restores the old way of speaking position in a list.
Trevor, what change the new patch? For example after I applied the last patch, and I moving Firefox file menu items, the spokened position index values is partialy right, only the 13th menu item position index value is not right. This menu item have before the quit menu item, this menu item spokened position index value is 1 of 2, not 13 of 14. For example in Thunderbird edit menu I think the spokened position index values is not right when a message is opened, the first menu item spokened position index value is 6 of 10, the second menu item position index value is 1 of 1, etc. You confirm this? The Thunderbird message list spokened position index values is full right. Possible fix this problem with XUL menues related? Attila
Created attachment 179833 [details] [review] use Gecko's posinset and setsize attributes Gecko provides us with attributes posinset and setsize for objects in lists and trees, for example message entries in thunderbird. We should use these attributes instead of figuring out the size of the group and the current elements position ourselves.
Trevor, now Firefox good the position index values for XUL menus, but Thunderbird I see different result. Look for example file menuitems position index values if not have opened any message, the position index value is sometime not increasing. For example the new menu item have 1 of 4 position index value, the open a saved message position index value is 2 of 17, the save as menu item is 1 of 1, etc. Why happening this? Or this is right and unable to solve? When I looked after doing a git reset --hard, make and make install the produced position index values, the values is following: - New menu: 1 of 17 - Save an opened message: 2 of 17 - Close: 4 of 17 - Save as menu item: 5 of 17 Etc. Attila
Created attachment 180138 [details] [review] use Gecko's posinset and setsize attributes Gecko provides us with attributes posinset and setsize for objects in lists and trees, for example message entries in thunderbird. We should use these attributes instead of figuring out the size of the group and the current elements position ourselves.
Trevor, this patch now result good position index values both Firefox and Thunderbird XUL menues, and resulting good position index values for Thunderbird message list. Only need fix following problem with end of patch, need insert an empty line. Need this because I try apply your patch, I get following error: patching file src/orca/scripts/toolkits/Gecko/speech_generator.py patch unexpectedly ends in middle of line
Trevor, your Thunderbird version you get always right position index values for mail folders tree view? Possible I not get right values because I have got too many tree level subfolders. Interesting, the 3.0.10 and 3.1.7 Thundervird version is shorting different order the folder names in the tree view. If not need fixing this, and not need doing you another work this bug related, I think we ready. If this patch not need doing any fixes, possible committing this patch if you inserting a blank line with end of patch?
(In reply to comment #42) > Trevor, your Thunderbird version you get always right position index values for > mail folders tree view? yes, I'm testing with a fairly recent nightly build. you don't? > Possible I not get right values because I have got too many tree level > subfolders. Interesting, the 3.0.10 and 3.1.7 Thundervird version is shorting > different order the folder names in the tree view. The different order is some sort of thunderbird thing, may be they changed the way they sort between versions? Anyway orca has no effect on that. Are you saying you get incorrect positions in the mail folder tree? If so can you reproduce this with a current build, and if so with what folder structure. > If not need fixing this, and not need doing you another work this bug related, > I think we ready. If this patch not need doing any fixes, possible committing > this patch if you inserting a blank line with end of patch? I can fix this if people with commit rights want, but it seems like its probably simpler for them to deal with it when they push.
Created attachment 180388 [details] Thunderbird treeview perhaps wrong position index values Trevor, I sending a debug.out file with shows me Thunderbird treeview position index values. I using Thunderbird 3.1.7 version. This index values are right? Attila
(In reply to comment #44) > Created an attachment (id=180388) [details] > Thunderbird treeview perhaps wrong position index values > > Trevor, I sending a debug.out file with shows me Thunderbird treeview position > index values. I using Thunderbird 3.1.7 version. what about with a nightly build as I asked before? > This index values are right? I have no idea, Please do as I asked before and explain The message folder structure you have, and what the indices are reported as and what you think they should be. > > Attila Thanks!
Attila?
Trevor, very sorry, I forgot the treeview problem looking with latest version prewious. I looked now this problem with latest awailable Thunderbird 3.3a3PRE version. The problem is happening only if I expanded a mail folder treeview with have subfolders. If I navigating the subfolders with an expanded treeview, Orca spokening the subfolder position index value. For example, my infoalap treeview folder have two subfolders, dexteszt and szakcsoport. If I expanded the infoalap subfolder, and jump the dexteszt subfolder, Orca spokening 1 of 2 position index value. This is right I think, because dextest is an expanded element with infoalap folder. Hopefuly I describe english right with I see. If I close all expanded treeview, Orca spokening continous right position index values (1 of 12, 2 of 12, 3 of 12, etc). This happening both Thunderbird 3.1.7 stable version and 3.3a3PRE version. Need handling your openion this treeview related problem, or this working method is right and Orca spokening right positionindex values with expanded subfolder treeview objects? Attila
(In reply to comment #47) > Trevor, very sorry, I forgot the treeview problem looking with latest version > prewious. > I looked now this problem with latest awailable Thunderbird 3.3a3PRE version. > The problem is happening only if I expanded a mail folder treeview with have > subfolders. If I navigating the subfolders with an expanded treeview, Orca > spokening the subfolder position index value. > For example, my infoalap treeview folder have two subfolders, dexteszt and > szakcsoport. > If I expanded the infoalap subfolder, and jump the dexteszt subfolder, Orca > spokening 1 of 2 position index value. > This is right I think, because dextest is an expanded element with infoalap > folder. > Hopefuly I describe english right with I see. > If I close all expanded treeview, Orca spokening continous right position index > values (1 of 12, 2 of 12, 3 of 12, etc). Well, I'd argue that that is the correct way to number the positions > This happening both Thunderbird 3.1.7 stable version and 3.3a3PRE version. ok, yes, since where geting the position in the set that is the set of elements at this level, so imho everything is working in both versions. > Need handling your openion this treeview related problem, or this working > method is right and Orca spokening right positionindex values with expanded > subfolder treeview objects? as I said I think this is correct, and the previous behavior was incorrect. Also I'm not sure how much we could do about this since like the message list this is a tree view so it might be hard to change the way we present the position. > > Attila
Comment on attachment 180138 [details] [review] use Gecko's posinset and setsize attributes http://git.gnome.org/browse/orca/commit/?id=508c1ae23a0d1ab385d6f6e6972fe28f0ce8cb86
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.