GNOME Bugzilla – Bug 468098
The whereAmI implementation doesn't always match the whereAmI spec
Last modified: 2008-07-22 19:33:13 UTC
I looked at the spec for whereAmI and compared it to what we are doing. Here is the situation: 1. Radio button groups were not always being announced in whereAmI, yet were when focus initially moved there, e.g. via Tab. For an example, look at the printing example in gtk-demo. Issue: whereAmI's _getGroupLabel() is only looking at panels. Labels for panels can also be found in fillers. 2. Mnemonics for combo boxes and sliders are not being announced. The issue seems to be that you can't associate a mnemonic directly with these objects, so instead you set a couple of properties on the label: use_underline and mnemonic_widget. This causes pressing Alt+the underlined letter to give focus to the object identified in mnemonic_widget. It also means that we can't use action.getKeyBinding() to obtain accelerators for combo boxes. Sliders don't even seem to have actions(??). 3. Spin button values are being announced twice (e.g. Copies: 2 spin button 2.0 Alt S). We should only announce it once. I checked with Mike and he indicated that we should speak the displayed value. The spec should be updated to reflect this. 4. We're not quite following the spec for menu items, especially not when looking at the example utterances. Example of what we should be saying: "File menu, Open..., Control O, item 2 of 20, O" What we are saying: "File, Open..., Control O, menu item, item 2 of 20, Alt Fo" 5. Tabs in a Tab list. The example in the spec is "Tab list, braille page, item 2 of 5." We're saying: "Page, item 2 of 5, braille page." I also discovered through the printing example in gtk-demo that we might have hidden page tabs (e.g. 2 showing, but 7 in existence). We aren't handling the case of hidden page tabs. 6. Tree Tables. We aren't speaking "expanded" for expanded items. In addition, it is not clear from the spec if "relative position" means relative to the entire tree or with respect to just the parent node. Mike indicated via email it means relative to the parent node. The spec should be updated to reflect this. Our current implementation is relative to the entire tree. I left icon panel alone because I know that Rich and Mike worked on that as part of the Nautilus script. The spec might need updating, however, to reflect the decisions made for Nautilus. In addition, it seems like label and role are repeated in the spec. Currently it says this: Icon Panels present the following information (an example is "Icon Panel, Mike Pedersen, item 8 of 10, "): 1. label, if any 2. role 3. label, if any 4. role 5. all selected icons followed by "selected I'm confused about the spec for "Detailed Information: Double Press of KP_Enter". Here is the current spec with my comments interspersed: When KP_Enter is pressed twice quickly, Orca will speak and display the information it currently does when performing the "Where am I" command, with the following changes: JD: I asked Mike if "speak and display the information it currently does" referred to the original implementation of whereAmI (which was extremely detailed and included all objects in the hierarchy from the top down to the current object as I recall) or if it instead referred to the current, single press. Mike said the latter. 1. Orca should pause briefly between items in the hierarchy JD: If it's not the original detailed view, what items in the hierarchy? If this refers to, say, the menu that contains a menu item, does this mean that the single press should *not* be pausing briefly but instead sticking everything together as a single string without pauses? If so, then I believe single press needs to be reworked to not pause. 2. Information regarding relative position should come after the item it pertains to, rather than before. (e.g. say/display "Euro Converter, Item 7 of 9") JD: If you look at the detailed spec for the single press, the relative position comes after the item it pertains to. Therefore, there is no difference that I can see between single press and double press w.r.t. position.
Created attachment 93922 [details] [review] the bulk of the changes This patch is the bulk of the changes. It does not include the change in trees where we speak the position relative to the children in this node (as opposed to the entire tree). I suspect that what needs to be done here will also need to be done to address 456970, so I'm going to deal with these together. In the meantime, Mike please test this patch and also provide clarification/guidance w.r.t. my spec questions. Thanks much!!
This patch does seem to make a lot of improvements. I'll address the remaining questions in another comment.
In an earlier conversation Joanie and I came to the belief that the current functionality of the double-press of the WhereAmI key isn't terribly useful. The doublepress is going to be reserved for special application functionality such as that required by Firefox and nautilus. I will update the spec shortly to reflect this change.
The spec has now been updated.
Created attachment 94088 [details] [review] more changes This version does what the previous version does plus: 1. Removes the double-click code that we're no longer using. 2. Removes "dramatic pauses" between a few object names and roles so that we're saying "yes radio button <pause>" instead of "yes <pause> radio button <pause>". I think this makes things smoother without creating a single glob of text, but I'd like feedback on this. 3. Adds speaking of roles and states for ROLE_CHECK_MENU_ITEM and ROLE_RADIO_MENU_ITEM. 4. Makes radio button state consistent with what we're doing in speechgenerator.py (i.e. "selected"; not "checked"). 5. Adds the default button identification code in, modified as per our discussion in team meeting. We're now identifying more default buttons than before (e.g. the default button in gcalctool's Insert ASCII Value dialog). Note that this requires the latest gail if you're in Feisty. 6. Checks in Gecko.py that we're in document content (as opposed to, say, a dialog box) before doing the page summary. I still need to do the tree table stuff (including the NODE_CHILD_OF handling). But in light of string freeze -- default button handling has a new string -- I thought I would attach this for consideration now. Thanks!
This patch works great for what it is intended to fix. I know you still want to do trees but the menus and radio buttons look good. I still can't get the default button patch to work for me but that's another issue.
Thanks Mike. Did you try this patch on Gutsy? (Without using the original default button patch). Will, we've got several days until string freeze. So.... Pending review (please review).... Should we check this in now and make the proper announcement to the lists, or just include it for post 2.20? Thanks!
It worked on my gutsy a few days ago but I can't test it right now because my gutsy is trashed again.
(In reply to comment #7) > Thanks Mike. Did you try this patch on Gutsy? (Without using the original > default button patch). > > Will, we've got several days until string freeze. So.... > Pending review (please review).... Should we check this in now and make the > proper announcement to the lists, or just include it for post 2.20? Thanks! > I think this looks pretty good, especially if it works. ;-) The only thing I'm leery of is this: + if defined: + fullShortcut = "Alt " + label.text.getText(i, i+1) I think we might need to be careful about the "Alt " string due to localization concerns. There are translatable strings for Alt in our code already (see keybindings.py -- "Alt_L" and "Alt_R"), so I'd opt for using the "Alt_L" string. Other than that, looks good to check into trunk and gnome-2-20 branch. I didn't have a chance to test, though -- I just reviewed. I'm hoping Mike and you did the testing coverage. In any case, it seems like a vast improvement over what we had.
Will thanks for the feedback! As you suggested, I tried using Alt_L. That caused Orca to announce the shortcut as Alt_L x (where x is the underlined letter). I don't think that's ideal. We could at least turn it into "Left Alt" by keynames.getKeyName(), but then there's the question of "Why does Orca say 'Left Alt' for shortcuts for combo boxes and sliders and 'Alt' for the rest?" I'll give it some more thought, but I think it would be nice if Orca were consistent with how it reports the shortcut.
(In reply to comment #10) > Will thanks for the feedback! > > As you suggested, I tried using Alt_L. That caused Orca to announce the > shortcut as Alt_L x (where x is the underlined letter). I don't think that's > ideal. We could at least turn it into "Left Alt" by keynames.getKeyName(), but > then there's the question of "Why does Orca say 'Left Alt' for shortcuts for > combo boxes and sliders and 'Alt' for the rest?" I'll give it some more > thought, but I think it would be nice if Orca were consistent with how it > reports the shortcut. > Yeah, it's unfortunate that combos and sliders seem to be broken when it comes to mnemonics. It would be ideal if they were consistent with the rest of the toolkit, and it makes me wonder if you're trying to go through great pains to work around a bug in GAIL. If this is indeed a bug in GAIL (I see that it doesn't seem to have accelerator support for comboboxes or sliders), I would almost opt for not working around this problem in Orca. Just filing a bug against GAIL and opening a new blocker regarding the missing mnemonic functionality for the broken components might be the thing to do. It seems like it should be easy to fix: gailbutton.c and gailentry.c seem to have example code that could be used. If there is a very compelling need to have this functionality now, however, I suppose you could try to use the gtk call gtk_accelerator_name to obtain the string much like how GAIL does it. But...we probably should really fix the problems at their source.
Created attachment 94391 [details] [review] latest version I ripped out the mnemonic hack. Also added a Q_() that was needed. I think this one is good. For some reason Mike cannot get the default button check to work on Feisty. I just wiped my Feisty box, so I'll re-check once everything is reinstalled. However, last time I checked, just adding the latest gail was enough to get it working. Looking at some debugging output from Mike, we're registering the double-click, and we're getting the new IS_DEFAULT state, so I'm not sure why it's not working there. In the meantime, I will be checking this in to squeeze under the string freeze wire.
Just fixed the Q_() in both trunk and the 2-20 branch. Sorry for being such a dip today! :-(
While working on the patch for bug #480021, I figured I'd tackle the last of the things which I believe need doing w.r.t. this bug, namely speak the position of items in a tree table relative to the node rather than the whole tree. I didn't like the result. Given a structure like this: Item 1 subitem 1a subitem 1b subitem 1c Item 2 subitem 2a subitem 2b subitem 2c If I am on, say, subitem 1b and am told that's item 2 of 3, my conclusion is that I'm in a list of 3 items and I've got one more to go. I would *not* automatically conclude that I've neared the end of the entire structure. On the other hand, if I'm on that same item and hear "row 2 of 3", I would conclude that I'm near the end of the entire structure. "Row", to me, refers to my location with respect to the entire table whereas "item" seems far less global/absolute. My suggestion is that we leave whereAmI tables as currently implemented. I called Mike for his opinion, and he said that was fine. Other thoughts or should we move this one to pending?
> My suggestion is that we leave whereAmI tables as currently implemented. I > called Mike for his opinion, and he said that was fine. Other thoughts or > should we move this one to pending? I'm short on time so have trouble looking at the code right now. What happens in the case of this kind of list: Verified Bugs Rich Joanie Scott Eitan Willie Pending Bugs Rich Joanie Scott Eitan Willie Assigned Bugs Rich Joanie Scott Eitan Willie If you were on the item "Pending Bugs->Joanie", do we provide enough information to let you know you're in the "Pending" branch?
We would say "cell, Joanie, row 9 of 17, tree level 2" Is that enough information? Perhaps, perhaps not. Maybe we should hear all of the ancestors? Mike? Here's what the spec says: Tree Tables present the following information (an example is "Tree table, Mike Pedersen, item 8 of 10, tree level 2"): 1. label, if any 2. role 3. current row (regardless of speak cell/row setting) 4. relative position relative to the parent 5. if expandable/collapsible: expanded/collapsed 6. if applicable, the level 7. all other selected rows (regardless of speak cell/row setting) followed by "selected" We say "cell" instead of "tree table." I had asked Mike about that when I was originally looking at this and he said "cell" was correct. Mike, your thoughts?
I still prefer cell but I don't feel really strongly about it so if anyone feels differently I can be talked into changing my mind.
Joanie, will this one require a string change or addition? If so, we might need to push it to 2.21.5.
The only item this bug is depending on that I can do anything about is bug #486897. I don't believe that will need a string change. Everything else in this bug is (I believe) already done. If I'm missing something that I'm supposed to do for this bug, please let me know. Thanks!
Closing this bug -- we can track the last remaining one (bug #486897) all on its own. Thanks for all the work on this one!