GNOME Bugzilla – Bug 161271
Spinbutton values read wrong by speech & brlmon
Last modified: 2006-07-27 13:50:11 UTC
Using gnopernicus 0.9.17 - Start gnopernicus and enable speech and braille monitor - On the main menu, navigate to and select: preferences->DesktopPreferences-> Display->Desktop Background. - Under the dekstop colours section, activate the pick a colour pushbutton. The Pick a colour window appears. - Navigate (using <tab>) to the 'Green' spinbutton. The current value should be highlighted. - Press <left> The focus will be at the start of the string - Type in a new value. If the old value was 56, and the entered value is (for example) 78, then the displayed value will now be 7856 with the caret positioned after the 8. If the user presses the delete key the current value will be 786. However, the speech & braille monitor still say the value is 7856. Press delete again and the output will be the desired value (e.g 78) but the gnopernicus output will still be 786. This bug cannot be reproduced on Linux builds.
I managed to reproduce this bug on Linux builds also. It doesn't occurs always (on both Linux and Solaris) but sometimes it is reproductible (often on Solaris and sometimes on Linux). I've checked also with an at-spi independent tester, to obtain the text line contained in the spin button (in its Text interface). The results were the same: sometimes the text seems to not be actualized after a char was deleted. The strange (inconsistent) behavior is that after an 'object:text-changed-insert' the reported text in always the updated one (in tester and gnopernicus). But after 'object:text-changed-delete' the text seems to not be always updated. Why this difference? It can be also observed with at-poke: if you insert a char the text is automatically updated but if you delete a char it isn't.
You shouldn't call the AccessibleText APIs when inside a text-changed handler; the result is not defined.
In gnopernicus when srlow module decides to present an event to user, that event is stored in an internal structure. Then an idle is set. When the idle callback is called the event is fired to srcore which presents the event to user. So, the AccessibleText APIs are not called inside a text-changed handler. Probably the bug can be reproduced easyly on slower computers. A suggestion is to try to compute the text for a text-delete event inside the handler. This is easy for a spin-button (a single line text) but almost impossible for other objects (a multi-line text like in gedit) when the text deleted belongs to more than 1 line. A compromise is to do this only for spin-button and only for text-delete event. Computing the current text also implies: 1. to do it in event handler -> more time spent in handler (only in this case) 2. store the computed value in sro structure -> a new field in that structure 3. report the computed text in case of asking for text line for spin-button -> a particular case for spin-button for text-delete event. Another solution is to delay somehow the presentation for spin-button in case of text-delete event. This way, the spin-button will have the chance to update its information. The best solution is (for all objects, not only for spin-buttons) to have text-delete events fired after updating proper information. This is the case of text-insert events. The text deleted can be now obtained from the event by asking for changed text.
Remus said: .A suggestion is to try to compute the text for a text-delete event inside the .handler. This is easy for a spin-button (a single line text) but almost .impossible for other objects (a multi-line text like in gedit) when the text .deleted belongs to more than 1 line. I don't see why it is more difficult, since the text content from any given AccessibleText object is relatively easy to cache. Why doesn't the same algorithm work for all text objects?
I suggest: 1) gnopernicus cache the text content of the currently-focussed object (and the watched object) if it implements AccessibleText, as part of its cache/state info about the current SRObject. 2) gnopernicus can use the text-change string and offsets from the text-change event to modify the cached text, inside its idle-time event handler. So no increased processing is required in the event handler, and the event can safely be processed at idle, using the cached text content. WHen focus changes (or a watch changes), the appropriate text cache can be stored, for later updating with the text-change data.
Bill - for gedit (one big text object across multiple lines), I think your algorythm will work. For something like gecko or yelp or StarOffice, the text change can be across multiple text objects (though of course only one of them will be "currently-focused"). At a minimum any algorythm implementation needs to be tested in those situations.
The multiple object case is not relevant, since a text-changed event covers one object only. That's my point - anything with multiple text objects will only emit a change on the object that has actually changed. If you delete text across multiple objects, you get multiple change notifications.
Bill, things are not so simple. In example below: | = left/rigth margin oftext area _ = " " (space) = no text (like after 3333) (seen as spaces, but nothing there) |11111111111_22_| |3333 | with caret after 22. By deleting the space after 22 (by pressing DELETE), the new content is: |11111111111_ | |223333 | with caret beetwen 22 and 3333. So, while gnopernicus tries to compute the new text line inside the handler the current line is "11111111111_22_". It deletes last "_" and reports "11111111111_22" as being the new content of the new line. This is wrong, because the caret moves to the next line and the REAL new line is now "223333". So, as I said before is almost imposible for gnopernicus to compute the new line. As a conclusion, biggest problems are when text migrates from a line to another after deleting a part of it. This is not the case of single-line texts. In this case is pretty easy to compute the new content.
Created attachment 37280 [details] [review] proposed patch for single line texts
Remus: In your example, gnopernicus can certainly keep the text content up-to-date. The problem you report is not a problem with the text content, but with the fact that the line breaks have changed. But in this case, gnopernicus will receive a caret-moved event as well as a text-change event. It's true that the caret-moved event doesn't contain line information, so it could be difficult to get consistent line information when this event occurs, for the reason you state. Perhaps we need some sort of 'text-reflow' event to let gnopernicus know that text linewrap has changed, in the future. I don't see this as a problem that is relevant to this bug though, because this bug concerns only those objects for which the entire text content (or current line) is re-read when text changes. I don't know of any multi-line text widgets which one wants to re-read on each text change event - for multi-line widgets the desired behavior is different, in most cases you want to read/present only the change to the user, not the entire content or even the current line. So I believe that the single-line patch will work fine for all the presentation types of interest, for instance spinbuttons.
The line content is presented for braille (and brlmon too) for all events for objects with text interface. Speech presents only the difference (obtained very easy from event). If the text-delete event will be fired _after_ updating the content, then the 'text-reflow' event will not be necessary. Text-delete event is fired before because of historical reason. In the past, the only chance to obtain the deleted text was to get the text from a range (event's details), because function AccessibleEvent_getChangedText was not present at that time. It was introduced later. Bill, what do you think about having text-delete event after updating the text content?
The async behavior of the text widgets is, in general, out of our control. I don't see this as a problem for the presentation types under discussion.
Bill, what do you mean by 'async behavior of the text widgets'?
widgets may update their 'view' asynchronously from all other operations. Even the model updates may be asynchronous. We don't have any control over whether the model updates or view updates are synchronous with keyboard events or even gtk+ signals, therefore we cannot guarantee that the ATK events delivered (which must be linked to the gtk+ signals) are synchronous with the updates to the underlying text data. Basically it means that we cannot insure that the text content is updated before the text-delete event is fired.
Then, this bug may be present in case of text-insert also? (never seen, but possible...)
This is certainly possible; code should be ready to deal with this situation too. I think we should update the patch to deal with insert situation too.
In some cases (perhaps all, I never seen a case where not) when the text-delete event is received, the text content is already updated. It _contains_ the new text. By forcing the insertion, the new text will be present twice in the current line. So, even if that is possible (at least in theory), to create a patch seems to be very difficult. Because the text is updated before firing text:insert event, I still wonder why that is not possible in case of text-delete too.
I disagree with your conclusion, it should not matter whether the text content has been updated in the widget or not, you should be using the cache which was obtained when the object was first focussed/navigated to, and is updated whenever text is inserted or deleted. In order to keep the cache current, it is necessary to handle both insert and delete events in this manner.
Created attachment 37839 [details] [review] proposed patch for gail to solve this problem This patch does same thing as it is done for text-insert event.
This solves the problem for one object type (gtkentry) in one toolkit (gtk+). It's an interesting and elegant solution, but I think it still leaves us vulnerable (we cannot always guarantee that we can make the notifications coherent with the object state in this way).
GtkEntry is used by all objects which have a single line entry field in gtk (those I know about). I checked for spin-buttons, combo-boxes, entry-fields.
I still think there's a general, underlying problem, which the text-change-event 'any' data (giving sufficient info to keep the local cache consistent) is intended to solve. However I don't see why the gail patch, or a version of it shouldn't go into gail.
Bill, are we talking about the general case of text-change events? Or about the text-change events for single line texts? Keeping a consistent cache for general case is very complex (gnopernicus has to compute the text line in case of deleting/inserting text at the end of the current line. Also the problem of having attributes updated to display them on braille is present in this case. If we are talking only about single-line texts, then, for all objects using gtk the problem is solved by the patch proposed for gail. SO sends events asyncronously, so, when the event is received the text is update (I assume). So, in my oppinion having the problem solved in some cases in the toolkit and in some cases in gnopernicus will lead us to toolkit specific code.
Remus said "So, in my oppinion having the problem solved in some cases in the toolkit and in some cases in gnopernicus will lead us to toolkit specific code." That's another reason, in my opinion, for solving the problem in gnopernicus, for the general case, and not relying entirely on the toolkit-specific gail patch.
I opened two bugs related on this one. The first: bug #170324 The second (for gail): bug #170327
Closing, since all problems here are subjects of other bugs.
See my comment #22 above. Applying the patch means that the text-change-event's "Any" data doesn't get filled properly, so this patch may need to be reverted. Clients should be using the 'any' data to obtain the change string.