GNOME Bugzilla – Bug 440490
Key bindings should allow double and triple press features to be rebound
Last modified: 2008-07-22 19:33:29 UTC
Suggestion from Will in comment #24 of bug #435226 as a better way of helping the user know what all the Orca commands are: "... we could add a "click count" field to a keybinding. That way, each discrete piece of functionality would be a separate key binding, each with its own learn mode text. This would be somewhat involved, however, and cause changes across the code and to the key bindings."
Does anyone have an opinion on this? Since it would be a rather significant change, we probably should consider it sooner than later if we want to do it for GNOME 2.22. I personally would like to see it, but it has the potential to introduce output latency for the first press of keys that also support double click. Note also that it potentially could help resolve bug 487226. I'm not 100% sure, but I think that the hardcoding of the "/" was to handle double click or something like that for laptop layout.
In my opinion this would be a nice to have but not at the cost of performance.
The advantage of a "click count" field extends beyond documentation/learning: Someone with a physical disability might not be able to reliably double-click. Therefore, I'm wondering if there's a way we could have our cake and eat it too: attend to/check for the possibility of a double/triple-click but don't actually wait for it.
(In reply to comment #3) > The advantage of a "click count" field extends beyond documentation/learning: > Someone with a physical disability might not be able to reliably double-click. > Therefore, I'm wondering if there's a way we could have our cake and eat it > too: attend to/check for the possibility of a double/triple-click but don't > actually wait for it. This might be possible and has the potential to clean things up a fair amount. One way to approach this might be to add a click count field to the keybindings.KeyBinding class, perhaps making it an optional argument to the constructor (default value = 1). Then, script.py would probably need modification somewhere in the {consumes,process}KeyboardEvent methods to 'do the right thing' based upon the click count. This would also mean moving default.py:getClickCount to script.py, which is probably the right thing to do. The preferences UI would also need to be modified to have a click count column.
Created attachment 99508 [details] [review] take 1 Since this interests me, and since no one else has claimed it, I decided to give it a go. The attached is take 1. Its goal is to allow us to "have our cake and eat it too," namely avoid the output latency Will referred to. > Then, script.py would probably need modification somewhere in the > {consumes,process}KeyboardEvent methods to 'do the right thing' based upon the > click count. Tried this, but couldn't quite work it out. So, Will, if you wouldn't mind giving me another hint as to what you mean/what I should do, that would be awesome. In the meantime, I took a different approach just to get something fully working. The approach is this: 1. Set orca_state.clickCount at the same time we set orca_state.lastNonModifierKeyEvent, only set it for key press events. 2. Place the burden on identifying the appropriate inputEventHandler on keybindings.getInputHandler(). I *believe* the resulting behavior is quite close to (if not the same as) what we currently do. This also seems to cause {consumes, process}KeyboardEvent to "do the right thing" -- or perhaps more accurately, it seems to prevent them from being able to do the wrong thing. > This would also mean moving default.py:getClickCount to > script.py, which is probably the right thing to do. Removed from default.py. For now, I put a one-liner in script.py which returns orca_state.clickCount. > The preferences UI would also need to be modified to have a click count column. Done, sorta. I added two hidden click count columns. Double-click and triple-click is indicated as appropriate in the text column for the keybinding or alternate. To set a binding which has a double-click or a triple click, you double-click or triple-click during the key capture process. The ability to set click counts means that a user could intentionally or accidentally quadruple-click -- or more! That seemed bad. Therefore, this patch decrees that the maximum number of click counts shall be three; no more, no less. (Five is right out.) Anyhoo, this patch is rather long and I'm really sorry. But this isn't an uber patch: It focuses on the task at hand. It's merely a sizable task. For anyone who wants to beat on it, I *believe* it's already in beatable-on (beat-onable?) form even though it's only "take 1." Thanks!!
I had a quick look at this: Firstly, I expected to see a separate column with the click count (not the words "(double click)" or "(triple click"), in with the key binding. Secondly, I triple mouse clicked in the "KP_2 (double click)" field (pretend I'm a low-vision user), just behind the "e" in double, with the intention of replacing "double" with "triple". (Yes, I know this isn't how it works, but visually a new user might think that it works that way). The text wasn't selected. I pressed the backspace key expecting just the character to the left of the text cursor to be deleted. The whole entry was deleted. Thirdly, I tried the same thing again. After backspace cleared the field, I typed "2" on the numeric keypad, and heard "the key entered is already bount to Speaks, the current flat review character. Trying to type KP_2 twice or three times quickly (i.e. a double or triple click) generated the same error message. How would I change this to be a triple click key?
> Firstly, I expected to see a separate column with the click count > (not the words "(double click)" or "(triple click"), in with the > key binding. I toyed with that and thought it was confusing, but I could certainly do that if that seems better. Based on your thoughts, perhaps it does. :-) My rationale was this: 1. Really wide trees are not ideal. Assuming that the keybinding and alternative binding can each have a unique click count, we want to have column headers that clearly identify them. While "1", "2", and "3" don't need much room, "Alternative click count" does. 2. Will splitting the keybinding into two columns confuse users or make it more difficult for them to obtain the desired info efficiently? I thought it might, but that's why Mike is in charge of UI. <smile> 3. Consistency with how things currently work. Currently we capture keys; we don't edit strings. > How would I change this to be a triple click key? You have to click really fast three times. ;-) Okay, so it looks like it's back to the drawing board. ;-) But to answer your question: 1. Press Enter to begin capturing. 2. Press KP_2 three times in rapid succession (just like you would to use such a binding) 3. Press Enter to confirm. Thanks!
Just saw your last statement. I should read more closely. Currently, it will start the complaint if there's a single click already bound (just like we would sometimes say that dynamic row headers were being set and then stop to say that they were being cleared). Perhaps a cleverly-placed speech.stop() would help there. Regardless, when I try it, if I click sufficiently quickly 3 times it works. Are you seeing something different?
Mike, spec please. <smile> Thanks!
> Regardless, when I try it, if I click sufficiently quickly 3 times it > works. Are you seeing something different? Ah. If I do it *really* quickly, then it does bind it to triple click.
> > 2. Will splitting the keybinding into two columns confuse users or make it more > difficult for them to obtain the desired info efficiently? I thought it might, > but that's why Mike is in charge of UI. <smile> I agree that splitting this information into two columns would be more confusing. >
Thanks. That leaves determining how the user should specify the number of clicks? Should one actually have to click the number of times or should one do something else?
Sorry for the spam, but a while back I wrote an admittedly long RFE (see bug 498008) pondering how necessary alternative keybindings are at this point. If they're still necessary then never mind. If it turns out they are not however, that would simplify the tree considerably and then we could indeed have a separate click count column.
I'd prefer to have the person press the key the number of times required. That said, do you think this could cause confusion for the user if they think they pressed the key three times but only two were registered? I'm really open to ideas on this one.
(In reply to comment #14) > I'd prefer to have the person press the key the number of times required. Initially that's what I was thinking as well (or I wouldn't have done the patch that way <smile>). Part of my rationale was that we don't want a complicated tree. The other part of it was that it gives the user a feel for exactly how quickly one must "click" (which currently is pretty quickly). If you can't click three times that quickly, perhaps a triple click binding ain't for you. <shrugs> By the same token.... > said, do you think this could cause confusion for the user if they think they > pressed the key three times but only two were registered? Indeed I do. See Rich's comment 6 and comment 12. If it's not intuitive for an Orca developer, how intuitive will it be for an Orca user -- especially a novice user? > I'm really open to ideas on this one. I am as well because I really don't know what the right answer is. But as soon as it's provided, I'd be happy to implement this RFE. <smile>
(In reply to comment #15) > (In reply to comment #14) > > I'd prefer to have the person press the key the number of times required. > > Initially that's what I was thinking as well (or I wouldn't have done the patch > that way <smile>). Part of my rationale was that we don't want a complicated > tree. The other part of it was that it gives the user a feel for exactly how > quickly one must "click" (which currently is pretty quickly). If you can't > click three times that quickly, perhaps a triple click binding ain't for you. I agree with this approach. Lets stick with it because I really don't think any other approach will be better.
Is any further info needed from me on this one?
I tried applying the patch, but I think it's outdated now, causing a number of rejects: ./src/orca/default.py.rej ./src/orca/Gecko.py.rej ./src/orca/where_am_I.py.rej ./src/orca/keybindings.py.rej ./src/orca/orca_gui_prefs.py.rej Joanie made the argument in the team meeting that this is not just a documentation issue, but is also an issue for people with physical disabilities who might not be able to do double press and triple press operations. As a result of that discussion, I think we also need to adjust the priority and severity of this bug. I'm going to guess high/major for now.
(In reply to comment #18) > I tried applying the patch, but I think it's outdated now, causing a number of > rejects: My bad. I figured we'd still be okay. I am still figuring that it won't take much to make this trunk compatible. :-) So I'm retargeting for 2.21.91, but planning on doing it sooner rather than later due to the approaching string freeze.
Created attachment 103987 [details] [review] trunk compatible version This version should be compatible with trunk. I tested it functionally. It all pylints to 10.0. I have not yet regression tested it. This is a big ol' patch because we had to address the following: 1. Document the keybindings with multiple clicks. 2. Enable the user to change keybindings -- not just to something else, but to something which might be a double- or triple-click. 3. Separate out all of the keybindings which currently have mulitple clicks hard coded into them. 4. Make new methods to handle the newly-separated bindings (e.g. in StarOffice.py, we now need one method to set dynamic row headers and another method to clear them). As a reminder, to bind something to a double- or triple-click, you have to click that many times during the binding. And you have to click pretty quickly -- just as quickly as you would to execute that command: I didn't change anything w.r.t. timing. Perhaps we should ultimately consider a user-adjustable "click" interval. (In some other RFE for some other release. ;-) ). Also please note that if you change a binding and use a double or triple click in it, the relevant settings file will have a keybinding in it that is not compatible with the current Orca (because of the new click count). You can manually delete these bindings. At least that's what I do. Finally, this has a number of new strings in it. Freeze is 11 Feb. See "big ol' patch" comment above. ;-) Thanks!!
This one seems to work wel for me.
(In reply to comment #20) > Created an attachment (id=103987) [edit] > trunk compatible version > > This version should be compatible with trunk. I tested it functionally. It > all pylints to 10.0. I have not yet regression tested it. I tested this last week and found it to work well. I definitely like the exposure of the input method descriptions for double and triple click. Please commit if you're comfortable with this (i.e., you did regression testing with it) and don't forget to send out a string change announcement if you do check it in.
I ran the gtk-demo regression tests before and after with this patch and found some regressions. They need a bit more digging, but I want to put them here before I lost them. The learn mode one (the first regression) is expected, but there seems to be a general problem with braille and also with SayAll. Test 4 of 6 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/learn_mode.py:Orca command: flat review current word EXPECTED: "BRAILLE LINE: 'Speaks or spells the current flat review item or word.'", " VISIBLE: 'Speaks or spells the current fla', cursor=0", "SPEECH OUTPUT: 'Speaks or spells the current flat review item or word.'", ACTUAL: "BRAILLE LINE: 'Speaks the current flat review item or word.'", " VISIBLE: 'Speaks the current flat review i', cursor=0", "SPEECH OUTPUT: 'Speaks the current flat review item or word.'", Test 7 of 7 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_accel_label.py:Where Am I default button EXPECTED: "BRAILLE LINE: 'gtk-demo Application UI Manager Frame close Button'", " VISIBLE: 'close Button', cursor=1", "BRAILLE LINE: 'gtk-demo Application UI Manager Frame close Button'", " VISIBLE: 'close Button', cursor=1", "SPEECH OUTPUT: 'UI Manager'", "SPEECH OUTPUT: 'Default button is close'", ACTUAL: "SPEECH OUTPUT: 'UI Manager'", "SPEECH OUTPUT: 'Default button is close'", Test 3 of 3 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_alert.py:Interactive Dialog Where Am I EXPECTED: "BRAILLE LINE: 'gtk-demo Application Interactive Dialog Dialog Entry 1 Testing $l'", " VISIBLE: 'Entry 1 Testing $l', cursor=16", "SPEECH OUTPUT: 'Interactive Dialog'", ACTUAL: "SPEECH OUTPUT: 'Interactive Dialog'", Test 1 of 1 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_status_bar.py:Status bar Where Am I EXPECTED: "BRAILLE LINE: 'gtk-demo Application Application Window Frame ToolBar Open Button'", " VISIBLE: 'Open Button', cursor=1", "BRAILLE LINE: 'gtk-demo Application Application Window Frame ToolBar Open Button'", " VISIBLE: 'Open Button', cursor=1", "SPEECH OUTPUT: 'Application Window'", "SPEECH OUTPUT: 'Cursor at row 0 column 0 - 0 chars in document'", ACTUAL: "SPEECH OUTPUT: 'Application Window'", "SPEECH OUTPUT: 'Cursor at row 0 column 0 - 0 chars in document'", Test 10 of 10 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline.py:SayAll EXPECTED: "SPEECH OUTPUT: '", "The keyboard sure can get sticky.'", "SPEECH OUTPUT: '", "Tis this and thus thou art in Rome?'", "SPEECH OUTPUT: '", "'", ACTUAL: "", Test 51 of 100 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_5 2X to spell 'is' EXPECTED: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'is'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", ACTUAL: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'is'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", Test 52 of 100 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_5 3X to military spell 'is' EXPECTED: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'is'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: 'india'", "SPEECH OUTPUT: 'sierra'", ACTUAL: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'is'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: 'india'", "SPEECH OUTPUT: 'sierra'", Test 54 of 100 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_8 2X to spell 'This is only' EXPECTED: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'This is only ", "'", "SPEECH OUTPUT: 'T'", "SPEECH OUTPUT: 'h'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'o'", "SPEECH OUTPUT: 'n'", "SPEECH OUTPUT: 'l'", "SPEECH OUTPUT: 'y'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: '", "'", ACTUAL: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'This is only ", "'", "SPEECH OUTPUT: 'T'", "SPEECH OUTPUT: 'h'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'o'", "SPEECH OUTPUT: 'n'", "SPEECH OUTPUT: 'l'", "SPEECH OUTPUT: 'y'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: '", "'", Test 55 of 100 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_8 3X to military spell 'This is only' EXPECTED: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'This is only ", "'", "SPEECH OUTPUT: 'T'", "SPEECH OUTPUT: 'h'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'o'", "SPEECH OUTPUT: 'n'", "SPEECH OUTPUT: 'l'", "SPEECH OUTPUT: 'y'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: '", "'", "SPEECH OUTPUT: 'tango'", "SPEECH OUTPUT: 'hotel'", "SPEECH OUTPUT: 'india'", "SPEECH OUTPUT: 'sierra'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'india'", "SPEECH OUTPUT: 'sierra'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'oscar'", "SPEECH OUTPUT: 'november'", "SPEECH OUTPUT: 'lima'", "SPEECH OUTPUT: 'yankee'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: '", "'", ACTUAL: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'This is only ", "'", "SPEECH OUTPUT: 'T'", "SPEECH OUTPUT: 'h'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 's'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'o'", "SPEECH OUTPUT: 'n'", "SPEECH OUTPUT: 'l'", "SPEECH OUTPUT: 'y'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: '", "'", "SPEECH OUTPUT: 'tango'", "SPEECH OUTPUT: 'hotel'", "SPEECH OUTPUT: 'india'", "SPEECH OUTPUT: 'sierra'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'india'", "SPEECH OUTPUT: 'sierra'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: 'oscar'", "SPEECH OUTPUT: 'november'", "SPEECH OUTPUT: 'lima'", "SPEECH OUTPUT: 'yankee'", "SPEECH OUTPUT: ' '", "SPEECH OUTPUT: '", "'", Test 57 of 100 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_2 2X to spell 'i' EXPECTED: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 'i'", ACTUAL: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 'india'", Test 58 of 100 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_2 3X to military spell 'i' EXPECTED: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 'india'", ACTUAL: "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "BRAILLE LINE: 'This is only $l'", " VISIBLE: 'This is only $l', cursor=6", "SPEECH OUTPUT: 'i'", "SPEECH OUTPUT: 'india'", "SPEECH OUTPUT: 'india'", Test 100 of 100 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_Add to do a SayAll EXPECTED: "SPEECH OUTPUT: ' a test.'", "SPEECH OUTPUT: ' ", "This is only '", "SPEECH OUTPUT: '", "", "PLEASE DO NOT PANIC.'", "SPEECH OUTPUT: '", " ", "I'm just going to keep on typing.'", "SPEECH OUTPUT: '", "Then, I'm going to type some'", "SPEECH OUTPUT: '", "more.'", "SPEECH OUTPUT: ' I just do not know when to'", "SPEECH OUTPUT: '", "quit typing.'", "SPEECH OUTPUT: '", "", "I think I might have spent too much'", "SPEECH OUTPUT: '", "time in the lab and not enough time'", "SPEECH OUTPUT: '", "in the wild.'", ACTUAL: "", Test 1 of 1 FAILED: /export/home/wwalker/orca/trunk/test/keystrokes/gtk-demo/role_window.py:Window Where Am I EXPECTED: "BRAILLE LINE: 'gtk-demo Application GTK+ Code Demos Frame TabList Widget (double click for demo) Page ScrollPane TreeTable Widget (double click for demo) ColumnHeader Application main window TREE LEVEL 1'", " VISIBLE: 'Application main window TREE LEV', cursor=1", "SPEECH OUTPUT: 'GTK+ Code Demos'", ACTUAL: "SPEECH OUTPUT: 'GTK+ Code Demos'", Note also there are some other regressions that cropped up in the role_table.py tests that seem unrelated to this patch (and they might just be Solaris specific). I'm going to dig into those some more, but the general flavor of them is that there is an extra space at the end of the braille line when showing a row. For example, "....coke " instead of the expected "....coke". I haven't debugged where that might be coming from yet, bug I don't think it is this patch.
Created attachment 104765 [details] [review] Patch to fix SayAll and eliminate most of the regressions This patch is made against svnversion 3551. It fixed the SayAll problem and also addresses the missing braille in a number of the regressions. Something is still different with the 2X/3X presses for spelling, however, in that we're seeing extra braille being output. I'm not sure what's up with that (haven't debugged it yet), but it's bed time.
Created attachment 104787 [details] [review] Patch to fix remaining gtk-demo regressions This patch fixes the remaining regressions noticed in the gtk-demo tests. The issues were as follows: 1) There was an extra braille refresh being done (no biggy) 2) The role_text_multiline_navigation.py test was assuming a triple click did the phonetic/military spelling whereas the spec says a double click is for that and triple click is undefined. So, in fact, this patch fixes a bug we had. Note that this is a very large patch. Other things it has affected need to be tested. This includes the dynamic row/col set/clear support in StarOffice, the various where am I stuff in StarOffice and the various where am I stuff in Firefox. If we can get some good testing coverage in those spaces, I'm willing to accept the risk associated with getting such a large patch in so late in the game. PS - This also doesn't cause conflict with the contracted braille patch in bug 354470, so that's a good thing.
I did a combination of functional testing and things are working as expected. When I ran the regression tests for oocalc, I noticed that in one test (bug_363801.py) we sometimes were saying "Cell" before "A1" when closing one spreadsheet and giving focus to another, and other times we were not. Multiple runs of that test with and without the patch give inconsistent results (i.e. I cannot predict whether "cell" will be spoken or not based on whether or not the patch is applied), so I don't believe it's related to the patch. The patch to the multiline test fails for me.
Created attachment 104877 [details] [review] Patch that works with trunk (In reply to comment #26) > I did a combination of functional testing and things are working as expected. Thanks! > When I ran the regression tests for oocalc, I noticed that in one test > (bug_363801.py) we sometimes were saying "Cell" before "A1" when closing one > spreadsheet and giving focus to another, and other times we were not. ... > I don't believe it's related to the patch. Doesn't sound like it to me, either. Either we're getting some unpredictable behavior from OOo or the test needs to be reworked to possibly listen for events closer to the output (if this is even possible). > The patch to the multiline test fails for me. I'm not sure if you mean the patch failed to apply or if the test failed to run w/o unexpected failures. I'm guessing the former? Here's a new patch against trunk. It should hopefully work -- I just ran it with svnversion 3554.
> I'm not sure if you mean the patch failed to apply or if the test failed to run > w/o unexpected failures. I'm guessing the former? Here's a new patch against > trunk. It should hopefully work -- I just ran it with svnversion 3554. > Sorry about that. Yes, the former. This one applies just fine. :-) Thanks! So... What's the plan? 2.22 or 2.24?
(In reply to comment #28) > > I'm not sure if you mean the patch failed to apply or if the test failed to run > > w/o unexpected failures. I'm guessing the former? Here's a new patch against > > trunk. It should hopefully work -- I just ran it with svnversion 3554. > > > Sorry about that. Yes, the former. This one applies just fine. :-) Thanks! > > So... What's the plan? 2.22 or 2.24? 2.22 if we do more testing with it. I'm guessing you did the proper functional testing to give good coverage of the script-specific changed. In particular: StarOffice: Insert+r and (Insert+r)x2 to set/clear dynamic column headers Insert+c and (Insert+c)x2 to set/clear dynamic row headers Where am I for speaking paragraph Where am I for speaking table cell Where am I for speaking calc cell Where am I for speaking title Where am I for speaking status bar Gecko: I'm not sure of the exact impact of the changes in the script, but I'm guessing doing the various where am I operations: KP_Enter, (KP_Enter)x2, Orca+KP_Enter and (Orca+KP_Enter)x2. If you can give me sanity checking on these, I'd feel good enough to accept the risk. Right now, I'm embarrassed that we don't provide a user with any alternative to the double-press keystrokes. The patch for this bug is an important fix for that.
Created attachment 104888 [details] [review] Yet another patch against trunk which also pylints well This patch works with the latest trunk (svnversion 3556) and pylints well.
It also passes my updated FF regression tests. :-)
I'm retitling this bug since the real problem is that we should not be requiring double and triple presses (i.e., time based features) as the only means to get to functionality. This is in the realm of a Section 508 issue.
(In reply to comment #32) > I'm retitling this bug since the real problem is that we should not be > requiring double and triple presses (i.e., time based features) as the only > means to get to functionality. This is in the realm of a Section 508 issue. > See (f) of this (thanks Joanie): http://www.access-board.gov/sec508/standards.htm#Subpart_c
Patch committed, will send out string change announcement soon. Moving to pending.
This seems to work well.
Thanks. Closing as FIXED.