GNOME Bugzilla – Bug 546941
Extraneous focus: events issued by OOo Writer lists cause excessive chattiness and braille issues
Last modified: 2008-10-17 00:33:49 UTC
Please describe the problem: in oowriter, when we land on a list, we speak the list name, and the currently focused listitem. The braille display repeats the list name, followed by the role. Steps to reproduce: 1. open oowriter 2. type: A test. 3. select test 4. goto format menu, character 5. assume we wish to change the strikethrough, so tab across to it. 6. we hear "strikethrough list (without) strikethrough list" and on the braille display we see "strikethrough strikethrough list" Thanks Actual results: Expected results: Does this happen every time? yes Other information:
will see if i can do anything about this myself.
Created attachment 116234 [details] [review] revision 1 attached patch removes the duplication of label and displayedText. When we jump between lists, we seem to get the ListItem getting the focus before the List itself. I assume this is also happening for the braille display, but the listItem info flashes past too fast. So we only end up seeing the name of the list. Ideally we would like to see the name of the list, and the currently selected item. I tried to get the currently selected item from the list, or check which child has focus, but no success. Thanks
Will, If you could please commit the above patch for 2.24 that would be great. All the above patch does is varify that we are not displaying the same text twice. As for solving the reported bug, I dont think it is ours, oowriter seems to give several events when landing on a list (and possibly in the wrong order). Someone with accersizer knowledge should please look at these multiple events and why they are fireing.
(In reply to comment #3) > Will, If you could please commit the above patch for 2.24 that would be great. > All the above patch does is varify that we are not displaying the same text > twice. Since this touches a sensitive spot in the code, I'd need to really give it some thorough testing to make sure the regression tests pass. I'll start some now, but I'm not sure I'll make the code freeze deadline. We can always shoot for 2.24.1 if I don't make it today.
I just did a quick run of the regression tests in Firefox specifically related to lists and list items and didn't catch anything. Yea! That said, I look a closer look at the patch and had some questions: 1. If this is specifically to address an OOo issue, should the change be in the custom braille generator for OOo rather than the generic/default braille generator? 2. The change is not being made to the braille generator for lists or list items, but to _getDefaultBrailleRegions(). This concerns me a bit. Granted I'm prone to paranoia <grin>... That means we really need to test this well. 3. While I suspect it is highly unlikely, is there any chance of the label and the displayed text being the same thing on purpose? Hypothetically, say a combo box whose label is "Month" and whose default value is "Month" with all of the items beneath it being actual months. I see this sort of nonsense on the web so apparently someone thinks it's a good idea. In that case would this patch cause the displayed "Month" to not be shown? (if this doesn't apply to combo boxes, pick some other hypothetical widget). 4. With respect to this: > Ideally we would like to see the name of the list, and the currently selected > item. Maybe you did all the right things in your attempts and OOo is just broken. Wouldn't be the first time, nor the last. But.... Maybe some clever foo could be done in a OOo-specific braille generator for lists (or list items -- I need to really look at the bug to know). I, of course, defer to Will and Mike on, well, pretty much everything. But my gut is telling me that maybe we can address the problem more to your liking (name of the list, selected item) and do so in a more targeted way. Tell you what: I've got beaucoups of day job stuff to deal with atm, but I'd be happy to look at this further tonight or tomorrow night, see what I can come up with, and if I succeed write up exactly what I did and why I did it. That would also give you a patch to muck with so you can see how things work. Lemme know.
Created attachment 120249 [details] [review] Something to play with... Jon: 1. Ultimately we should look at making the changes you proposed in your patch, but in the meantime... 2. This patch creates a _getBrailleRegionsForList() within the soffice braille_generator. This means that the worst case scenario is that we are breaking braille only for OOo and only for lists. 3. This patch also displays the selected item. Things that have selectable children (menus, lists) should implement the accessible selection interface. If so, then you can ask for the nth selected child. All of the lists I've come across in OOo are not multiselectable, therefore I think it's safe to assume that that the first selected child (i.e. 0) is the right one. 4. This patch has far, far more comments than code. It shouldn't. Those are there temporarily for two reasons: 1) For me to answer in advance any "why the heck did she do *that*?" questions you might have, and 2) Because I honestly wasn't sure which role should be initially displayed. See the comments for my reasoning. :-) So.... If you still have the time and interest, I think this patch is in good enough shape for you to play with, see how things work, try out the different role possibilities, test it, etc. Feel free to change as you see fit. Assuming that you do have changes, please nuke my conversational/train-of-thought comments in your revision of the patch. Thanks!! As a related aside: While working on this, I noticed that the size combo boxes aren't being spoken. This is true with and without the patch in 2.4 and 3.0. YAOOB undoubtedly. I think some events have gone missing.... I'll add it to my to-do list.
Adding Jon's name, removing Will's. I'll leave testing required in case Mike wants to give it a try.
Created attachment 120613 [details] [review] Proposed patch Because Will plans to do the 2.24.1 release tomorrow, I decided to assign this bug to myself. Jon, I owe you one bug of your choice from my list -- should I have a bug which you're just dying to tackle, that is. <smile> This version does the following: 1. Removes all the comment verbiage. 2. Changes the new soffice _getBrailleRegionsForList so that it only does the list-related stuff (i.e. it does *not* cause the selected list item to be displayed) because we can do better: 3. Adds a check to onFocus to see if the focus: event we get happens to be for a list which is the parent of the current locusOfFocus. What's happening is that we're getting an object:active-descendant-changed event for the list (so we speak -- and braille -- the list item correctly). Then we get a focus: event for the list which causes us to speak and braille the list. The fact that we do this for speech means we're too chatty and re-speaking the list name after the list item; the fact that we do it in braille makes it appear that we are failing to display the list item. Simply ignoring this event causes the bulk of the problem to go away. #2 above causes us to not double-display stuff. 4. Updates a couple of regression tests: One is an existing test in which we were double-displaying a list label/name. The other is a test I wrote specifically for this bug and checked in earlier day, showing where things were broken. Already pylinted and regression tested. Will, please review. Thanks!
Chatted with Will about this via IM. He said commit. Done. Moving to pending.
Joanie, thank you for all that :) slowly getting back into gear (irritating hardware issues) Thank you for the commented version :) the commited patch is good, ideally we would have wanted to see the list label too, but seeing the current selected value is good for now. i see we are ignoring some events for lists, but I am still hearing events being fired and orca deciding to act on some of them, can we kill off any of the remaining events somehow? As for picking up one of your bugs :) i will remind you once i have tackled some of the existing bugs on my todo list. Thank you
(In reply to comment #10) > the commited patch is good, ideally we would have wanted to see the list label > too, but seeing the current selected value is good for now. It should be there -- along with the role, but to the left of the focused region. If that's not ideal, feel free to work with Mike to spec out what changes need to be made and/or make the changes in a patch of your own. > i see we are ignoring some events for lists, but I am still hearing events > being fired and orca deciding to act on some of them, can we kill off any of > the remaining events somehow? Yup. But those are for combo boxes and this was a "get it done now" patch to address the problem you identified. I'll investigate the combos further. I've also marked those as issues within the regression tests so each time we run the tests we'll be reminded. > As for picking up one of your bugs :) i will remind you once i have tackled > some of the existing bugs on my todo list. Deal! :-)
(In reply to comment #11) > It should be there -- along with the role, but to the left of the focused > region. If that's not ideal, feel free to work with Mike to spec out what > changes need to be made and/or make the changes in a patch of your own. aah yes, sorry, it is just that the focused region is at the left of the display (40 cells) and one has to activally navigate left to see that info. > > i see we are ignoring some events for lists, but I am still hearing events > > being fired and orca deciding to act on some of them, can we kill off any of > > the remaining events somehow? > Yup. But those are for combo boxes and this was a "get it done now" patch to > address the problem you identified. I'll investigate the combos further. I've > also marked those as issues within the regression tests so each time we run the > tests we'll be reminded. excellent, thank you
(In reply to comment #6) > Something to play with... > Jon: > 1. Ultimately we should look at making the changes you proposed in your patch, > but in the meantime... > 2. This patch creates a _getBrailleRegionsForList() within the soffice > braille_generator. This means that the worst case scenario is that we are > breaking braille only for OOo and only for lists. > 3. This patch also displays the selected item. Things that have selectable > children (menus, lists) should implement the accessible selection interface. If > so, then you can ask for the nth selected child. All of the lists I've come > across in OOo are not multiselectable, therefore I think it's safe to assume > that that the first selected child (i.e. 0) is the right one. > 4. This patch has far, far more comments than code. It shouldn't. Those are > there temporarily for two reasons: 1) For me to answer in advance any "why the > heck did she do *that*?" questions you might have, and 2) Because I honestly > wasn't sure which role should be initially displayed. See the comments for my > reasoning. :-) > So.... Hi Joanie, I know :) a little late, but still hoping to learn we have: obj.getState(), obj.querySelection() and other things related to object. If i try the following in an IPython console, i dont see these functions, where are we getting them from? $ ipython in: import gtk in: import atspi in: list = gtk.List() in: li1 = gtk.ListItem('foo') in: li2 = gtk.ListItem('bar') in: list.add(li1) in: list.add(li2) in: list.querySelection() #fails, no such attribute in: list.getState() #also fails I must be missing something obvious, any hints greatly welcome :) Thank you
getState() and querySelection() are methods associated with Accessible objects; not Gtk+ objects. Were I you, I'd give Accerciser's iPython console a try and work with the actual, Accessible objects Accerciser presents rather than attempting to create brand new objects. Hope this helps!
Jon opened bug 556476 on the not speaking the Size combo box. I opened bug 556657 for the combo box chattiness. Therefore I'm closing this bug as FIXED. Jon if you are interested in changing Orca so that the list label is also visible when a list item is displayed, feel free to open a new bug for that. Thanks!