GNOME Bugzilla – Bug 463223
sayAll for Gtk+ text widgets should try to keep the text that's been spoken visible.
Last modified: 2018-02-08 12:26:53 UTC
Summary pretty much covers it. As you do a "say all" on a document that's larger than what's visible on the screen, the an effort should be made to keep the text that's been spoken visible. This would be for low vision users.
Created attachment 93159 [details] [review] First attempt at this. The attached patch will now update the caret as the "say all" progresses through the text. As Joanie points out, it's probably doing too much updating at the moment. Perhaps it should be adjusted to only update every nth progress feedback (where n is to be determined), or some other way. The real reason for filing this bug though was to get it to always show the text caret position, so if you've got to the bottom of the visible text, then the text would automatically scroll up, and updating would continue. In testing this with gedit, evolution and OOo Writer, only the later nicely does this. The other two quite happily update the text caret position off-screen. So two question (probably aimed at you Will): 1/ What should n by for the showing of the new text caret position? I'd say about every five calls to __sayAllProgressCallback() when in PROGRESS state, but I'd like to see what others think. 2/ What's the best way to get it to automatically scroll up, say half or a full windows worth, for applications like gedit and evolution? Is there an at-spi call we can use here, or do we have to fake up something like a Page Down event?
Created attachment 93163 [details] [review] proof of concept patch: change gail! :-) Lunch break time! :-) Rich, I looked at gtk-demo's text examples and we don't scroll there either. So then I looked at gail. it seems that set_caret_offset() just places the caret at the textIter. Doesn't do any scrolling. BUT we can make it scroll with a one-line change. :-) If you get gail from svn trunk, apply this patch, and reboot, your patch will scroll the distance required to get the textIter on the screen *in gedit and gtk-demo*. Doesn't seem to impact Evolution; haven't tried to hunt down why. Anyhoo, this is just a proof of concept/for whatever it's worth thing. I'm not convinced that the attached patch is THE answer. But I do think that the answer involves a change to gail. After all, IMHO, it's rather silly to set the caretOffset without regard for whether or not that position is visible. :-)
Nice. I actually prefer a half page scroll when it needs to be visible, to make it work in a similar fashion to OOo Writer. Index: gailtextview.c =================================================================== --- gailtextview.c (revision 1277) +++ gailtextview.c (working copy) @@ -566,6 +566,7 @@ gtk_text_buffer_get_iter_at_offset (buffer, &pos_itr, offset); gtk_text_buffer_place_cursor (buffer, &pos_itr); + gtk_text_view_scroll_to_iter (view, &pos_itr, 0.0, TRUE, 0.0, 0.5); return TRUE; } I'll investigate why evilution isn't doing the correct thing. Probably got there own version of a text view or something. Thanks!
I tried this version with Gedit and it scrolled almost constantly, about a line at a time, keeping the text in the middle of the window. I think we should only be scrolling if the text is not visible to make it easier for low vision users to track what's on the screen. My personal preference is not a half page scroll either. :-) <shrugs> What I was thinking in my proof of concept is this: If gail is modified so that it scrolls the minimum amount necessary to ensure that setting the caret causes the caret location to be visible, then ATs can do stuff on their end to determine the scroll amount. For instance, knowing that we have such a minimalistic scroll, we might decide to set the caret offset to the next paragraph during a sayAll. Admittedly hacky, which brings me to: What would be really cool is if set_caret_offset() took a couple of additional arguments: * how much to scroll by * when to do the scrolling: * Always * Never * When the text at the caret offset isn't visible. That would not only allow ATs to set the amount to scroll in a less hacky fashion, but it would allow ATs to allow the user to set the amount to scroll by. Then you can set Orca to scroll half a page always (line with focus will therefore be always dead center) and I can set it to scroll a couple of lines only when the caret offset isn't visible. This latter approach would, I assume necessitate a corresponding change to atk (?) and to the at-spi docs, etc., etc. and might take longer. Although, ya know, it might be worth the wait...
> I tried this version with Gedit and it scrolled almost constantly, about a line > at a time, keeping the text in the middle of the window. I think we should > only be scrolling if the text is not visible to make it easier for low vision > users to track what's on the screen. You're right. What we need in the gail code is something like: if text not currently in view: do a half page scroll I'll see if I can work out how to change this appropriately.
Well, I'm a tad confused. I've been trying to work out the gtk+ code for: if text not currently in view: do a half page scroll I thought it should be: Index: gailtextview.c =================================================================== --- gailtextview.c (revision 1277) +++ gailtextview.c (working copy) @@ -566,6 +566,10 @@ gtk_text_buffer_get_iter_at_offset (buffer, &pos_itr, offset); gtk_text_buffer_place_cursor (buffer, &pos_itr); + if (gtk_text_view_get_cursor_visible(view) == FALSE) { + printf("It's not visible. Scroll to it.\n"); + gtk_text_view_scroll_to_iter (view, &pos_itr, 0.0, TRUE, 0.0, 0.5); + } return TRUE; } but when I run it (testing with gedit), every time it tries to do: gtk_text_view_get_cursor_visible(view), I get a: (gedit:6245): Pango-CRITICAL **: pango_layout_get_cursor_pos: assertion `index >= 0 && index <= layout->length' failed I'll keep looking...
Created attachment 93201 [details] [review] alternative gail patch (from Joanie). Joanie, in her tenacious terrier persona, has been worrying away at my problem, and has come up with an alternative patch that will check if the text caret cursor is currently in the visible portion of the document and do a half page scroll if it isn't. Now we need to decide: a) if this kind of fix should go into gail. b) if yes to a), then which is the better solution: 1 line or half page scroll. Joanie also worked out that gtk_text_view_set_cursor_visible() wasn't what I wanted here, because it's inverse void gtk_text_view_set_cursor_visible (GtkTextView *text_view, gboolean setting); Toggles whether the insertion point is displayed. A buffer with no editable text probably shouldn't have a visible cursor, so you may want to turn the cursor off. (All I can say is that the online Gtk+ documentation at www.gtk.org does a poor job of describing gtk_text_view_set_cursor_visible()). Thanks for all your help Joanie. I think I'll wait until it's been decided how we wait to fix this, before generating the next Orca patch. If we are going to do an "update cursor position every nth time that the __sayAllProgressCallback() is called in the PROGRESS state), then there is definitely some tidying up of the code in all the __sayAllProgressCallback() routines that can be made.
> Joanie, in her tenacious terrier persona, Uhhhh.... Thanks??? ;-) > Now we need to decide: > > a) if this kind of fix should go into gail. > b) if yes to a), then which is the better solution: 1 line or half > page scroll. At the risk of reinforcing the notion that my persona in any way resembles said canine.... ;-) c) if instead of *just* a) and the subsequent need to decide b) on behalf of any AT that might want to (already does) call setCaretOffset(), should at-spi, atk, and gail be modified so that setCaretOffset()/set_caret_offset() takes two OPTIONAL arguments as described in comment 4. They would default to the current behavior (i.e. no scrolling, scrolling amount == 0.0) so as not to impact current calls to the method(s). This would allow AT -- and if the AT implemented such support -- USERS of AT to configure their own scrolling behavior to fit each unique situation (sayAll versus some yet-to-be-thought-of feature) and personal preference. (Apologies to Rich who's already heard me say the above twice via email.)
>> Joanie, in her tenacious terrier persona, > > Uhhhh.... Thanks??? ;-) I meant it as a compliment. > c) if instead of *just* a) and the subsequent need to decide b) > ... Sorry, I should have listed that one too. The problem here is that it might be scrollable here in both the X and the Y direction, and you might want different value for each. If we did this, we almost would want to replicate several of the parameters to gtk_text_view_scroll_to_iter() gboolean gtk_text_view_scroll_to_iter (GtkTextView *text_view, GtkTextIter *iter, gdouble within_margin, gboolean use_align, gdouble xalign, gdouble yalign); Scrolls text_view so that iter is on the screen in the position indicated by xalign and yalign. An alignment of 0.0 indicates left or top, 1.0 indicates right or bottom, 0.5 means center. If use_align is FALSE, the text scrolls the minimal distance to get the mark onscreen, possibly not scrolling at all. The effective screen for purposes of this function is reduced by a margin of size within_margin. Note that this function uses the currently-computed height of the lines in the text buffer. Line heights are computed in an idle handler; so this function may not have the desired effect if it's called before the height computations. To avoid oddness, consider using gtk_text_view_scroll_to_mark() which saves a point to be scrolled to after line validation. text_view : a GtkTextView iter : a GtkTextIter within_margin : margin as a [0.0,0.5) fraction of screen size use_align : whether to use alignment arguments (if FALSE, just get the mark onscreen) xalign : horizontal alignment of mark within visible area yalign : vertical alignment of mark within visible area Returns : TRUE if scrolling occurred -- Or maybe we, the AT, need to do the scrolling separately rather than add this to gail...
> I meant it as a compliment. No worries. I was just teasing. Besides if the fur fits.... ;-) > it might be scrollable here in both the X and the Y direction, and you might > want different value for each. True.... Good point! > Or maybe we, the AT, need to do the scrolling separately rather than > add this to gail... Looking at the at-spi idl, I suppose we can figure out if the character at the caret is visible or not -- if nothing else -- presumably via text.getRangeExtents() and component.contains(). But I'm not seeing anything promising once we decide that yes, indeed, we need to scroll to make that text visible. The only thing I can think of is to look for a scrollbar and if found try to set it's value. Using Accerciser to examine gedit's scrollbar, the method to set a scrollbar's value is marked as "private".
We just discussed this in the Orca team meeting today. The plan is to open up a separate rfe against gtk+, such that when you set the text caret to a new position and that location is not visible, then it's automatically scrolled into view. I will then block this bug against that (adjusting the summary to better reflect what's going on). We will also poll the low-vision folks on the Orca mailing list to ask them what they would like to see w.r.t. showing "say all" progress through a text document.
I've filed an enhancement request against Gtk+ for this functionality. See bug #464468. http://bugzilla.gnome.org/show_bug.cgi?id=464468 Blocking this one (and adjusting summary slightly).
Joanie, I think you are the best person to answer this one. Should a gail bug be opened for the patch you've attached below, or is the bug I opened against Gtk+ (bug #464468) the preferred way to fix this? If it's the latter, then the gail patch should be obsoleted. Note that Mathias Clasen thinks bug #464468 is a duplicate of bug #79216 (for labels). That bug has been open for over five years so don't expect too much progress on this.
Obsoleting my current patch. Not convinced that the call of "dup" is valid....
A week ago I added a comment to bug #464468 (which is, as Rich noted, closed as a dup). I don't think it's a dup and explained my rationale in the comment that I added. No response so far. Removing my name from the summary of this bug as I don't think there's anything else I can do at this point. :-(
Comment from Matthew to bug #464468 ------- Comment #6 from Matthias Clasen 2008-06-02 21:30 UTC ------- This should not be very hard to do, since as you point out, entries and text views already have all the mechanism in place to do it. For text views, scroll_to_iter/scroll_to_mark should indeed allow you to do what you want. I don't think this can be handled 'inside gtk', necessarily. At least, we can not unconditionally force the text cursor to never leave the visible range. It would have to be driven by the AT (which already sets the cursor position, as far as I understand the above discussion). I guess simply calling gtk_text_view_place_cursor_onscreen after each cursor move might be sufficient. I'll give this a try. Unblocking for now.
(In reply to comment #16) > Comment from Matthew to bug #464468 > > ------- Comment #6 from Matthias Clasen 2008-06-02 21:30 UTC ------- > This should not be very hard to do, since as you point out, entries and text > views already have all the mechanism in place to do it. For text views, > scroll_to_iter/scroll_to_mark should indeed allow you to do what you want. > > I don't think this can be handled 'inside gtk', necessarily. At least, we can > not unconditionally force the text cursor to never leave the visible range. > It would have to be driven by the AT (which already sets the cursor position, > as far as I understand the above discussion). I guess simply calling > gtk_text_view_place_cursor_onscreen after each cursor move might be sufficient. > > I'll give this a try. Unblocking for now. > Cool. Thanks Rich! Just a random thought before I head to bed... I'm not sure we can do this inside Orca since we don't have direct access to the text widget to call gtk_text_view_place_cursor_onscreen. Instead, the fix might belong somewhere in GAIL? The gail_text_view_set_caret_offset method of http://svn.gnome.org/viewvc/gail/trunk/gail/gailtextview.c?view=markup looks like it might be a candidate, but I'm just guessing. In addition, since GAIL has moved into GTK+ for GNOME 2.24 (I think), it'll need to be fixed there as well: http://svn.gnome.org/viewvc/gtk%2B/trunk/modules/other/gail/gailtextview.c?view=markup. Li might be a good person to help us here.
Will is spot on here. We need to be able to do the equivalent of the gtk_text_view_place_cursor_onscreen() via the accessibility infrastructure. Li, any suggestions on how best to accomplish this? Thanks. PS: I'll leave it depending upon bug #464468 for now, but either that bug is going to have to be reassigned to gail or we will need to open up a new one and block this one again.
When I said not 'inside gtk', I really meant 'in gail'...
I think gail_text_view_set_caret_offset it the best place. Is gtk_text_view_place_cursor_onscreen sufficient for Orca? From the comments, I saw there are 1 line or half page scroll options.
If it can't be configurable (the best solution), then scrolling by 1 line would be less disruptive. I suggest reclassifying bug #464468 against gail and applying the fix there. Thanks.
Created attachment 112218 [details] [review] Revision #2. Hi Li. Unfortunately your latest patch to bug #464468 doesn't work. Also, the Orca world changed since I wrote the first attempt at a fix for this (see comment #1), so here's a new version. It should be enough for you to test this. Apply this patch to latest Orca. Build and install. It actually is a patch for three things: 1/ To provide this functionality for gedit. 2/ To provide this functionality for Evolution 3/ To provide this functionality for "generic" Gtk+ applications that have a multi-line text view. (Note to Joanie: we will need to provide a similar change in Firefox and Thunderbird too). To see how it should work, do the following: 1/ Start up Orca 2/ Start up gedit and open up a text document that is larger than the window size. A handy "trick" here, is to reduce the height of the gedit window. That makes it quicker to get to the part we care about. 3/ Give focus to the gedit text area, and press KP_+ (that's the "+" key on the numeric keypad). Orca should start speaking the text document from where the text cursor was. As it speaks it, it should be moving the text cursor. When it gets to the last visible line on the screen, it scrolls up, making the next line visible. And so on. Now where it's failing (even with your gail patch applied) is with a Gtk+ text view application. To see this, do the following: 1/ Start up Orca 2/ Start up gtk-demo and start the Text Widget -> Multiple Views demo. 3/ Give focus to the lower text view. To cut down on the amount of time it takes to get to the interesting part, I suggest initially moving the text cursor to the beginning of the line that starts with "Colors. Colors such ..." 4/ Press KP_+ (that's the "+" key on the numeric keypad). As before, Orca should start speaking the text document from where the text cursor was. As it speaks it, it should be moving the text cursor. Unlike the gedit example above, when it gets to the last visible line on the screen, the next line is not scrolled up to make it visible. Note that Evolution doesn't do the right thing either, but as we are focusing more on Thunderbird these days, I'm not too concerned about this. As longs as this works in Firefox and Thunderbird (and Gtk+ applications) we should be okay. Thanks!
Firefox and Thunderbird don't use Gtk+ text widgets. In addition, when you position the caret Firefox and Thunderbird automagically scroll. And we've been repositioning the cursor in the progress callback In taking a look at it just now, it seems to work well given proper paragraphs and the like in HTML. It could use some refinement in the face of non-proper paragraphs (i.e. when the "paragraphs" are just text in the document frame itself separated by line break chars. So in a bugzilla bug or a orca-list message, in other words, we need to improve. If you try it on the Orca wiki or in an HTML formatted message (as opposed to a plain-text one) in Thunderbird, you should see how it works. The trick is not moving the caret for every character, and also not accidentally clicking on focusable objects, so we're currently looking at the offset and the role type. Regardless, the areas in which we could improve is a different bug from this one.
Okay, I just opened bug 536837 for the plain text issues in FF and thunderbird with a target for the 2.24 release.
I just tried this patch in Gedit. It looks like we're moving the caret for each word. Try the following: 1. Create a Gedit document with lots of long lines of text (i.e. the text should be allowed to flow, wrapping to the next line automatically as if they were traditional paragraphs) 2. Maximize the Gedit window so that it occupies the full screen. 3. Set up Orca with the following preferences: a. Magnification level 6x b. Magnification tracking and alignment text cursor centered c. Speech rate 70 4. Having set up the above, do a sayAll in your Gedit document. I think the screen is jumping around too much. By the time the user is able to visually focus on the newly-centered word, the caret's already moved again. We might need to consider looking at extents as well. And, yes, we'd need to do so in Gecko too. There, we don't have the jumping problem -- but we don't scroll to the right either.
(In reply to comment #22) > Hi Li. Unfortunately your latest patch to bug #464468 doesn't work. > Hi Rich, The patch works fine on my machine... Without your patch for Orca, the cursor jump to the end of the text after Orca finish the reading. With the patch, the cursor jump down line by line to the end of the text when Orca is reading. What is the situation on your machine? Does the cursor not move at all?
Once the cursor gets to the last visible line, it no longer moves and no new lines are scrolled visibly. Looks like I need others to try this. I'll comment to that effect in the Orca bug (bug #464468). Thanks.
That should have been gail bug #464468 of course.
I've just tried this with latest Ubuntu Intrepid and Gtk+ from SVN trunk plus your one line patch to .../modules/other/gail/gailtextview.c and it works just fine. I don't know why it didn't work for me before. Sorry for the false alarm. I'd say your gail patch is good for checking in. I'll add a similar comment to that effect on bug #464468. Thanks!
I've got some ideas bouncing around in my head on this. Plus I have a corresponding version of this bug for Gecko, so assigning this one to me with a target to match my Gecko one. Might as well attempt to do them together.
I just tried this patch with gedit on OpenSolaris 2008.11. It actually does give me the "follow the bouncing" ball effect by moving the caret on the screen. Pretty interesting. Braille also updates pretty frequently as a result, and I think we need feedback from our UI experts on that one. One thing I did notice was that it seemed to get a little confused when dealing with the repeated character collapsing code (I think). What I did was open the Orca README document and do a SayAll in it. When speech was near the line of = signs below the "Introduction" heading, the caret movement seemed to get pretty confused. It eventually got back in sync, though.