GNOME Bugzilla – Bug 523693
[pending]Positioning of the cursor when editing text
Last modified: 2009-11-09 21:35:18 UTC
Hi, When editing text, i.e., when inserting or deleting characters, the braille display is positioned such that the cursor is on the last cell of the braille display. This is a proble, because you cannot see surrounding of the place you are editing. Especially, when you are deleting text, you can only see the next character to be deleted (but not the word it belongs to) without panning the display. Therefore, I often delete more than I intended. When starting to edit text in the middle of the display, the display jumps to the position described above. In my opinion: * When editing text in the middle of the display, the display should not be repositioned * If, by inserting text, the end of the display is reached, the display should be positioned in the middle of the display. Then the context behind and infront of the cursor is visible. this is of course just my personal opinion. When editing text in the 1st window of a line (in my case 40 chars) Orca behaves correctly, i.e., the display stays put. This report applies both, to OpenOffice and to forms in FF3. Thanks alot for looking into it! Cheers, Rudolf
Hi, Here is an experimental patch which solves these problems. When "preserveCursorCell" is true, the cursor is kept at the same cell wen inserting/editing/deleting text (in particular, after the cursor was moved via cursor routing keys). If "preserveCursorCell" is true, stayAwayFrom[Left|Right]Margin can be used to make the cursor stay away from the display boundaries when editing text. This makes sure that you can alsways see the surrounding of the place you are editing. If these modifications tunr out to work (which is not at all certain as this is my first try at modifying the orca code), it would be nice if these features could be configured via keyboard shortcuts. I did some testing of the patch in OO and gedit. I did not test in FF due to bug #541762. One thing I would like to improve - though I have no idea how to do it - is to not preserve the cursor position when adding text to the end of a line, because then there is no point in keeping the cursor away from the right boundary of the display as there is no text to the right of the cursor anyway. I'd be greatful for some advice on that. Thanks in advance, cheers, Rudolf
Created attachment 114065 [details] [review] patch to braille.py obtained using "svn diff" in the working copy
Created attachment 114276 [details] [review] alternative patch Is this the sort of behaviour you were thinking of? The attached patch never seems to jump the display unnecessarily, and the whole display is always used. pylints to 10.00 Thank you
Adding "testing required" per Jon's request.
Hi Jon, thanks alot for your patch. I tried it out the last few days. I think, it is a big step forward. It solves the "cursor jumps unnecessarily" problem very well. In addition - probably as a side effects - many labels to form fields are now showing which were not visible before becasue the first cell of the input field was on the first cell of the braille display and hence the label was invisible. I have no clue at all, how the patch does this, but I can't say I completely understand all the different kind of offsets involved in th setfocus()-function :-) What we still need, in my opinion, is a solution to the "editing at the boundary of the display" probelm - i.e. backspacing when on the 1st cell of the display or inserting/deleting at the last and not seeing your surrounding. I'll have a look at your code and try to include that. Thanks alot again. cheers, Rudolf
Hi Rudolf, Sorry for not replying sooner. I'm glad that you think it is an improvement. As for the editing on boundery problem, i wasnt quite sure what would be best. what about the following: let X,Y be posative numbers. if cursor < X and there is text to the left of the display then shift visible display by Y (to the left). if cursor > displaySize-X then shift visible display to the right by Y. Do you think a reasonable value for X could be 3? and Y could be displaySize/3? i.e. if the cursor is in the first 5 braille cells of the display and there is more text to the left, then move the display to the left by a third. similarly for the right edge of the display. The reason why i didnt do this before, is that i really dont like my braille display jumping around, and if i wanted to see a bit more then i would explicitly move it. But if you agree with the above then it would be intresting to see it work. Mike, your feedback would be greatly appreciated. Thank you
sorry for the typo above, first i had 5 everywhere, then i had 3, but forgot to change the example.
Hi Jon, for what it's worth I agree with you that the display should not jump around any more than absolutely necessary. Are you guys ready for me to test and review this one from a usibility perspective?
Created attachment 115438 [details] [review] update to alternative patch Ok, scrap my previous comment, hopefully this is better still. This patch: 1. when arrowing beyond the display boundaries, keep a third of the display of old content, and the remaining 2/3 is filled with new (possibly blank. 2. when backspacing (deleting chars) when cursor is at start of display, then move display so that we see 2/3 of the previous text (to the left) and a third of the content to the right. 3. When deleting (del) forward and the cursor is on the last cell of the display, move display by 2/3 to the right, showing us a third of the content to our left, and 2/3 of the content we are considering for deletion. tested in gedit and oowriter, pylinted to 10.00 Please test, comments welcome.
I'm curious about the overall usability of this -- it seems like it *might* be a bit jumpy, but I'm not sure and will defer to braille power users for their thoughts. As an alternative model, I wonder if we might consider something similar to the magnifier "push" approach. With this, the cursor would tend to try to be positioned where the braille display will always show some text to the left or right of the cursor, except when at the edges of the text. For example, assume I have a display whose width is 10 cells (just for discussion purposes). The model might work something like this, with more details to be worked out, of course. It could also all get screwy when dealing with right-to-left vs. left-to-right locales (assuming braille in right-to-left locales is written right-to-left, which I don't know the answer to): Navigation: 1) If I'm navigating left, I try to keep the cursor fixed at cell 3, always showing two characters to the left. If there aren't two characters to the left, then the cursor moves appropriately to cell 2 then cell 1. If the cursor is at a cell greater than cell 3 when I start navigating left, I just move the cursor left for each navigation action until it lands on cell 3. 2) If I'm navigating right, I try to keep the cursor fixed at cell 8, always showing two characters to the right. If there aren't two characters to the right, then the cursor moves appropriately to cell 9 then cell 10. If the cursor is at a cell less than cell 8 when I start navigating right, I just move the cursor right for each navigation action until it lands on cell 8. Inserting text: 1) If I'm inserting/typing at the end of the string, I try to keep the cursor fixed at cell 10, except of course if the string is less than 10 characters. In that case, I show the entire string and move the cursor to the right until I reach cell 10. 2) If I'm inserting/typing from the middle or beginning of the string, I start with the cursor at its current cell and work my way to cell 8 a character at a time, then the cursor remains fixed at 8. The assumption is that I want to be able to review what I just wrote vs. what's coming next. Deleting text: 1) As I delete text via backspace or delete, I attempt to keep the cursor at the current cell. With this, part of the display remains static (i.e., the side of the cursor I'm not deleting), while the other side moves (the side of the cursor I am deleting). The navigation/insertion stuff above should hopefully already have me in a position where I'm in a position to see text to the left and right of the cursor.
Jon, Will says you and I need to chat more on this one. Let me know what is a good time and I'll try to be on IRC.
Hi everyone, Sorry for the lon ginactivity, I was on holidays for the last weeks. I tried the new patch and it definitely improves the situation with respect to "standard" orca behaviour. However, I personally prefer Will's concept. This is also (more or less) what I tried to achieve in my initial patch. Regards, Rudolf
Hi Will and others, Thanks all for the feedback, Mike I will attempt to implement Wills requested behavior, so we dont need to catch up on irc for now.
(In reply to comment #14) > Mike I will attempt to implement Wills requested behavior, so we dont need to > catch up on irc for now. Mike and Jon: I am not a power braille user, so I only made a suggestion that seemed reasonable. You two are much much more proficient and reliant upon good braille behavior, so I really want to make sure the solution that is created results in an efficient experience. I will not be at all offended if you come up with something else.
Created attachment 116569 [details] [review] revision 4 Hi Mike, Could you possibly make Friday anytime between 12 and 4 GMT? I will be on irc most of the afternoon today, if you get to read this in time. Rudolf, thank you for your help, would you mind testing this patch? It should hopefully be behaving as you wished. If you wish to change how close the cursor comes to the edge of the display, change the marginBuffer = 2, both in setFocus and refresh functions. all: I have not made the previous patch obsolete yet, because I still think it is the less jumpy solution. (Consider an empty line, and we start typing a really long line, that requires the focus to be 2 cells away from the edge of the display while we type. I would consider the shifting of the display by one char for every keypress as a disturbing jump, and should be avoided. But thats just my personal view. Sorry for feeling so strongly about this.) Please test and looking forward to hit this one on the head soonish. Thanks all
Just to explain, me and Mike had a chat on irc, and the outcome was to instead of having a fixed border, we would move by whole words. when arrowing: always try to keep a whole word between the cursor and the edge of the display. "this is a test" if we can only see "a test" as we try to move onto "a" the "is" appears. Same goes for the other edge of the display. when deleting: "this is a test" and only "a test" is visable and the cursor is on the first t of test: as we backspace, we get "is atest", i.e. a new word has popped up between the word where the cursor is and the edge of the display. same goes for the other direction. Please stay tuned :)
Created attachment 117200 [details] [review] revision 5 Hi All, I think :) I've implemented The description for the required behaviour (previous comment). Please test, and thanks in advance for your comments.
Hi, thanks for the new patch. I just tried it out. Personally, I prefer the fixed-boundary behaviour, because then the display jumps only 2-3 chars. Word are often much longer. I find it hard to find the caet again, e.g., after a using cursor routing key - wich using this patch often causes a repositioning of the display. In sourcecode "words" may even become longer than the display: int x =function1(otherfunction(a,2,thirdfunction(x))); When entering such a string, the patch jumps between two states (tried in gedit): 1. Caret in the center of the dispaly (at the end of the string) 2. String fills entire display, but cursor is missing. The jumps between the two states apper after each consecutive character is entered. Maybe the two approaches could be combined by allowing for a maximum jump size. I.e. if making the entire next word visible makes a jump of 20 chars required, just jump 3-4. Jon, thanks for giving us the chance to test all these different approaches! Regards, Rudolf
Hi Rudolf, Thank you for that, Yes, sorry for not spotting the char-char jumping on long words. I will see if i can fix that, and also attempt to put in a maxJump limit.
sorry, forgot to ask, forgetting the long words behaviour for the time being, is everything else working as expected? Mike, ignoring the issue of long words, is this the behaviour we want?
Created attachment 121509 [details] [review] revision 6 this is exactly same as revision 5, but works with svn trunk. Thought i should put it here before i go and misplace it.
Created attachment 121689 [details] [review] revision 7 Hi, the attached patch: When we are within 'margin' of the display edge, then look to putting the full next word at the display. If the next word is too long, i.e. it is too far from where we were before, then only allow to jump by 'maxJump'. This also simplifies the code by only having the refresh function do the display positioning. Please test Thank you
Created attachment 121809 [details] [review] revision 8 After talking to delYsid (using 80 cell braille display) on irc, the conclusion is what is right for 40 cells may not be right for 80. * Modified the code so that we can easely add new panning styles. * added the word panning style. * added the brltty panning style. * added orca.settings.braillePanningStyle, which can currently be set to either orca.settings.PAN_BRAILLE_BY_WORD or orca.settings.PAN_BRAILLE_AS_BRLTTY. eventually we probably want to expose these (and other) panning modes using the gui too. Please test, comments welcome.
non-related to this RFE but, ./run_pylint.sh seems to have problems with braille.py mh@mh-vibex:~/builds/orca/trunk$ svn revert -R * ---omitted output--- mh@mh-vibex:~/builds/orca/trunk$ echo " " >>./src/orca/braille.py mh@mh-vibex:~/builds/orca/trunk$ make; sudo make install ---omitted output--- mh@mh-vibex:~/builds/orca/trunk$ ./run_pylint.sh Thank you for your attention to quality Checking braille.py, sending output to braille.pylint ./run_pylint.sh: line 16: 22589 Segmentation fault PYTHONPATH=$INSTALL_DIR: $PYTHONPATH pylint --init-hook="import pyatspi" $INSTALL_DIR/orca/$foo > $OUTPUT _FILE 2>&1 mh@mh-vibex:~/builds/orca/trunk$ pylint --version pylint 0.14.0, astng 0.17.2, common 0.30.0 Python 2.5.2 (r252:60911, Oct 5 2008, 19:24:49) [GCC 4.3.2] mh@mh-vibex:~/builds/orca/trunk$ python -c "import pyatspi" mh@mh-vibex:~/builds/orca/trunk$ Running ibex with latest updates, and pylint was installed using apt-get. should i build and install pylint and friends from source? Although all the version numbers provided are newer than those found at http://live.gnome.org/Orca/Pylint. Thank you
(In reply to comment #25) > Checking braille.py, sending output to braille.pylint > ./run_pylint.sh: line 16: 22589 Segmentation fault This is general problem on Ubuntu Intrepid. :-( As a side note, pylint works great on OpenSolaris.
Comment re patch revision 7 (which I tested a few days ago for Jon): There is a buglet in the patch, when in gnome-terminal, it can happen that the display doesn't show the full command prompt, but pans a few cells to the right, even though the display is large enough to fit the whole prompt. I think panning should only happen automatically if the text doesn't fit on the display.
Hi Mario, Thank you for the feedback, yes i can indeed see it too. after some thinking, the bug that we are seeing is due to gnome-terminal not giving us the right information. bug 397668 says that we always get lines of 80 chars, even if they are all spaces. this is causing the miss-calculation of the margins. The code should work well everywhere else though, and since it is the gnome-terminal that is broken, i am not sure if including a solution now is right, or to leave it as a separate patch. Will, what is best to do in this situation? Also if you have a bit of time to please have a look over patch 8?
Hi Jon - thanks tons for your work! Mario, thanks also for testing and providing informative feedback. (In reply to comment #28) > The code should work well everywhere else though, and since it is the > gnome-terminal that is broken, i am not sure if > including a solution now is right, or to leave it as a separate patch. > > Will, what is best to do in this situation? I think good terminal access through braille might be more important than the feature provided by the subject of this bug. But, I will defer to our users for their opinion on that. Ideally, we could get bug #397668 fixed and almost be done with it, though I fear users will still complain loudly if they need to upgrade to a new gnome-terminal/vte. Is there some way you can add a test for a line from gnome-terminal ending in all spaces? It might be an evil hack, but if you document it well and it solves the problems, it might be the better solution for our users. > Also if you have a bit of time to please have a look over patch 8 [edit]? From a quick surface level scan: These lines look like they are returning the opposite of what they mean to do: + def _getBraillePanningStyleString(self, braillePanningStyle): + """Returns a string that represents the braillePanningStyle passed.""" + + if braillePanningStyle == settings.PAN_BRAILLE_BY_WORD: + return "orca.settings.PAN_BRAILLE_AS_BRLTTY" + else: + return "orca.settings.PAN_BRAILLE_BY_WORD" Here, there is a trailing space in the key; is this intended? + elif key == "braillePanningStyle ": + value = self._getBraillePanningStyleString(prefsDict[key]) For the rest of the patch, there's a lot of code that definitely needs testing. I would feel comfortable if users gave this a hard test ride. I'd like to see them do things such as pan left/right through large blocks of text in gnome-terminal and gedit, use cursor routing keys, use flat review, etc.
I agree with Will here that doing something to solve the Gnome-terminal issue would be worth doing. I'm also now OK with the method here used to reposition the cursor.
Created attachment 122283 [details] [review] revision 9 This revision solves the gnome-terminal problem and fixes the typoes that Will picked up. further testing anyone? :) Thanks
I've tested this latest patch with both a 24 and a 32 cell display. What do you all think about getting this into trunk for wider testing?
Please don't commit this one yet. I think I've discovered a problem with it but am still checking.
(In reply to comment #31) > Created an attachment (id=122283) [edit] > revision 9 > > This revision solves the gnome-terminal problem and fixes the typoes that Will > picked up. I worked with this some today, and made a few local modifications to make it pylint better. When running this with the regression tests, I noticed that almost every one of them failed due to double braille-ing of many things. This seems to be due to the addition of a call to 'refresh' in the 'setFocus' method of braille.py. Was this additional call to 'refresh' (and the removal of calls to 'refresh' in other parts of the code) a needed part of the functionality for this enhancement?
(In reply to comment #34) > I worked with this some today, and made a few local modifications to make it > pylint better. Yes, sorry, still havent done anything about pylint in ibex, and trying to compile orca trunk on hardy (so that i can apply this patch) gives the intltool error. > When running this with the regression tests, I noticed that almost every one of > them failed due to double braille-ing of many things. This seems to be due to > the addition of a call to 'refresh' in the 'setFocus' method of braille.py. > Was this additional call to 'refresh' (and the removal of calls to 'refresh' in > other parts of the code) a needed part of the functionality for this > enhancement? The way I understood it: previously: both setFocus and refresh played with the position of the display. currently: the functionality is done in one place, refresh. Saying that, do we get better results if we remove the refresh call at the bottom of setFocus, and keep the refresh calls that i previously deleted? A quick test with a physical display, i didnt seem to notice anything breaking, what does the tests say? Thank you
(In reply to comment #35) > Saying that, do we get better results if we remove the refresh call at the > bottom of setFocus, and keep the refresh calls that i previously deleted? It works a lot better and eliminates many of the failures. I'll attach a new patch. > A quick test with a physical display, i didnt seem to notice anything breaking, > what does the tests say? Yeah - it's not really breaking, it's just making redundant writes to brltty, causing the braille tests to fail because of the duplicate output.
Created attachment 122890 [details] [review] Updated patch that pylints and reduces the number of test regressions Here's an updated patch that reduces the number of regressions and pylints to a 10.00. There are still test regressions that need looking at, though. I also do fear that this will have a significant impact on a number of tests because of the new alignment. We'll need to look at those, especially the Firefox tests.
Hi Mike, any hint on how to reproduce your bug? Thanks
Could someone please run the tests against this patch? I believe the problem I was seeing was related to something else.
(In reply to comment #39) > Could someone please run the tests against this patch? I believe the problem I > was seeing was related to something else. I suspect most of the tests will fail because it is going to change the alignment of the braille output. Have you applied the patch locally and run with it for a while?
I've been testing this against trunk from yesterday morning. So far I'm not having problems with it. I'd like to see it checked in for wider user testing.
The patch seems to work OK. Thanks Jon! There's a call to debug.printStack(debug.LEVEL_ALL) that probably should be removed, but that's easy enough to remove. In testing this with the gtk-demo tests, I see about 18 test assertions fail, which isn't too bad. Most of these are the result of cutting the rolename "Window" off the front of the visible braille area that is used for representing the small text area that appears when you press Ctrl+f to search for something in the main test window. This seems OK to me. The other assertion failures are the result of the the patch doing the right thing. :-) All in all, from a GTK+ perspective this looks OK. I'm curious what Joanie's experiences are with the Firefox tests. Over to you, Joanie!!!!!!!!! :-) !
Created attachment 126465 [details] FF regression test output Overall, I'm seeing the same sorts of things you are Will (i.e. cursor has moved). However, there seem to be some problems with certain form fields (the most notable being the dojo_spinner.py and line_nav_bugzilla_search.py tests). :-( Haven't had time to look yet. I'll try to do so this evening.
Sorry for the spam. The other thing I noticed is that there are a few cases where before we display the label when focus is first moved into a form field. With the patch, the user has to pan to the left to make the label of the newly-focused field visible: Test 4 of 15 FAILED: /export/home/jd/orca/test/keystrokes/firefox/label_guess_bug_509809.py:Next form field DIFFERENCES FOUND: BRAILLE LINE: 'Your email address: $l ' - VISIBLE: 'Your email address: $l ', cursor=20 + VISIBLE: ' $l ', cursor=1 SPEECH OUTPUT: 'Your email address: text' [FAILURE WAS UNEXPECTED] Test 5 of 15 FAILED: /export/home/jd/orca/test/keystrokes/firefox/label_guess_bug_509809.py:Next form field DIFFERENCES FOUND: BRAILLE LINE: 'Your name (optional): $l ' - VISIBLE: 'Your name (optional): $l ', cursor=22 + VISIBLE: ' $l ', cursor=1 SPEECH OUTPUT: 'Your name (optional): text' [FAILURE WAS UNEXPECTED] Test 6 of 15 FAILED: /export/home/jd/orca/test/keystrokes/firefox/label_guess_bug_509809.py:Next form field DIFFERENCES FOUND: [FAILURE WAS UNEXPECTED] Test 7 of 15 FAILED: /export/home/jd/orca/test/keystrokes/firefox/label_guess_bug_509809.py:Next form field DIFFERENCES FOUND: BRAILLE LINE: 'Reenter password to confirm: $l ' - VISIBLE: 'Reenter password to confirm: $l ', cursor=29 + VISIBLE: ' $l ', cursor=1 SPEECH OUTPUT: 'Reenter password to confirm: password'
> The other thing I noticed is that there are a few cases where before we display > the label when focus is first moved into a form field. With the patch, the user > has to pan to the left to make the label of the newly-focused field visible: > > Test 4 of 15 FAILED: > /export/home/jd/orca/test/keystrokes/firefox/label_guess_bug_509809.py:Next > form field > DIFFERENCES FOUND: > BRAILLE LINE: 'Your email address: $l ' > - VISIBLE: 'Your email address: $l ', cursor=20 > + VISIBLE: ' $l ', cursor=1 Eeks - this seems pretty bad. Jon, do you have any ideas?
From previous usage (gedit, oowriter, pidgin as examples), we have regions prepended to give contextual info. pidgin Application #orca Frame TabList Page ScrollPane Table jorcauser a: vp=61,of=61,to=61,ol=38,il=44,ir=79,or=81 This shows that the start of the display is at the 61th char of the string, i.e. showing the space before the name, and all of: "pidgin Application #orca Frame TabList Page ScrollPane Table " is context. Following the same logic, The regions on the same line in firefox are being used as context for the current active region, which is indeed true, the label is context information for the edit field. This way it is consistent, because the user always has to do a scrole left to get the context no matter where they are. It might be initially unappealing, but giving a braille only user a consistent environment is important. (Couldnt think of a quick easy way to fix it for firefox either, because we still use prepended regions for standard contextual info, such as in the dialogues). What do people feel?
Two comments here: 1. I prefered seeing the label before the edit field so as to have some context when looking at the display. This patch has a negative side effect as follows 1. go to a google search page. 2. type some text into the search box. 3. Tab forward to the go button. 4. Tab back to the search box. Notice that you hear nothing. This problem doesn't exist without the patch. Another site to test this is the sign-on page at www.comcast.net
This is potentially something we could do for 2.26.0. Jon, I'm curious if it might be able to make an option that allows people to go back to the "old" way if they run into problems with the new way.
Putting this on the 2.28 radar screen for now. Jon - can you work up a patch that allows Orca to continue to support the existing behavior so we can give users something to fallback to if they don't like the new behavior?
What's the status of this bug? does someone work on it? Thanks.
Sorry for the delay, thanks for pinging.
sorry for spamming, I wanted to look at this after the speech/braille refactor, and now that this is in master, I can revisit this bug.
Thanks a lot. Please let me know if you have something for testing.
Created attachment 138950 [details] [review] revision 11 Here we are again :) 1. fixes the problem of not being able to go back to fields with text already entered. 2. For all 3 methods (as brltty, by word, and as original) we can see the labels for fields in ff3 when we land on them. --- Using 'braillePanningStyle' we control the panning style to be used. PAN_BRAILLE_BY_WORD PAN_BRAILLE_AS_BRLTTY PAN_BRAILLE_AS_ORIGINAL Code contains debugging statements which will be removed if behaivour is reasonable. The default panning style is the original. to apply to git master: patch -p1 <filename Thank you for testing/comments.
Just putting my name on this so I'll remember to test it.
(In reply to comment #55) > Just putting my name on this so I'll remember to test it. Just a friendly reminder. Today is 2.27.5 tarball day.
My biggest comment on this one is that I'd like to see "word" as the default. That said if we could get that change I think it would be good to get this one in for wider user testing as I think only a couple of us have tested at this point.
I started running the gtk-demo regression tests with this patch with braillePanningStyle = PAN_BRAILLE_AS_ORIGINAL. Unfortunately, the VISIBLE lines all seem to be fairly different. :-(
Another thing that needs to be addressed with this (from Halim): Open a text editor and write something longer than 40 characters (70 chars). Press routing to go to 50. Char and delete it. The block-cursor should stay on pos 50 on the braille display. It jumps to the end of the brailledisplay and the user must search!.
Created attachment 139965 [details] [review] A different approach In looking at the prior patches, a lot of decision logic was placed in braille.py, and I think it might be better to put the logic in the script. This patch is an attempt at routing things through the script's _presentTextAtNewCaretPosition method, which is where we could jump off to other script code to do logic for where we want the caret to end up on the physical display (i.e., the target cursor cell). This patch also starts to address the funky jumping caret position issue in comment #59. That is, it seems to work OK for GTK+, gnome-terminal, and OOo, but Gecko is kind of elusive at the moment. If I can figure the Gecko stuff out and still manage to keep things routed to _presentTextAtNewCaret position, then I think we can start playing with different logic for setting the target cursor cell.
(In reply to comment #60) > Created an attachment (id=139965) [edit] > A different approach > > In looking at the prior patches, a lot of decision logic was placed in > braille.py, and I think it might be better to put the logic in the script. So, I went for a bike ride and thought about this. I now think some logic belongs in braille.py, too, since it will be the one to know how many cells there are and exactly what is being displayed for text.
Created attachment 140069 [details] [review] Patch to at least solve Halim's issue This patch seems to solve Halim's issue in comment #59. It pylints well and tests well with gtk-demo. I'm running the other tests now (fingers crossed). What this patch basically does is save information about what was being presented the last time we did a braille.refresh and compares it to the new stuff, adjusting the target braille cursor position as it sees fit. The other thing this patch does is identify a spot where we can do the realignment of the viewport and target braille cursor position. It's in braille.py as so: # [[[TODO: WDW - this is a potential spot where we might realign # the viewport and cursorCell to show complete words and such.]]] I'll work on this more tomorrow.
Created attachment 140138 [details] [review] Patch to build upon previous to add _realignViewport method This patch builds upon the previous patch to add a _realignViewport method to braille.py. The method works off new settings: # Braille alignment styles. These say how to align text on the # edges of the braille display. The brailleAlignmentMargin value # says how close to the edge of the braille the display the cursor # cell can get. The brailleMaximumJump says how far we can jump # the display when aligning by word. # ALIGN_BRAILLE_BY_EDGE = 0 ALIGN_BRAILLE_BY_MARGIN = 1 ALIGN_BRAILLE_BY_WORD = 2 brailleAlignmentMargin = 3 brailleMaximumJump = 8 brailleAlignmentStyle = ALIGN_BRAILLE_BY_EDGE The default alignment style matches what we have today. The ALIGN_BRAILLE_BY_MARGIN style uses brailleAlignmentMargin and is effectively a "push" model - when you get to the edge of the display, the viewport is pushed to keep the cursor cell at the margin (until you reach the edge of the text). The ALIGN_BRAILLE_BY_WORD style uses brailleAlignmentMargin in the same push model above, but when it pushes the viewport, it pushes it so the edge lands on a word boundary. In the event we hit a really long word, the brailleMaximumJump setting limits how far we jump. I'm not sure this is exactly what Mike and Jon are expecting, so feedback on this is desired. Please test. You can do do by setting orca.settings.brailleAlignmentStyle in your ~/.orca/user-settings.py file.
I committed the patch for 2.27.90 to allow it to get further testing. The default is to get us what we have today, but also fix the jumping cursor problem one experiences when deleting text in the middle of long strings. I'm going to leave this bug open pending testing and feedback of the WORD alignment option.