GNOME Bugzilla – Bug 439191
sayAll by sentence can position the text cursor in the wrong place when interrupted.
Last modified: 2008-07-22 19:28:14 UTC
This came out of the testing of bug #400716 – sayAll should be done by sentences. It's been isolated into its own specific bug. I will attach two files: 1/ Various Orca print debug statements to Orca in SVN trunk/HEAD, to help track down the interrupt problem. 2/ Orca run time output from these statements, when debugging the interrupt problem. Steps to reproduce: 1/ Start Orca. Make sure that we are doing "say all" by sentence. 2/ Start gedit showing the file in attachment http://bugzilla.gnome.org/attachment.cgi?id=87142 3/ Position the cursor at the beginning of the text file. 4/ Press numpad-Plus key to start the "say all" 5/ Use the Control key to interrupt the "say all". When doing "say all" by sentence the second "sentence" is: Introduction + [2]1.1 Features * [3]2. I hit the Control key just after the "2" in the following line had been spoken: + [2]1.1 Features The text caret was positioned at the very end of the same line instead of after the 2, after the interrupt. The text object that we are doing the "say all" on, is the whole of the file (74891 characters). The line in question: + [2]1.1 Features starts at offset 204 and ends at line 232 In the __speak() routine in gnomespeechfactory.py, there are calls like: text = self.__addVerbalizedPunctuation(text) if orca_state.activeScript: text = orca_state.activeScript.adjustForPronunciation(text) so what starts out as: Introduction + [2]1.1 Features * [3]2. ends up as the following text that is sent to the TTS: ` Introduction plus left bracket 2 right bracket 1 dot 1 Features star left bracket 3 right bracket 2.` The __idleHandler() routine in gnomespeechfactory.py is being called to provide progress feedback as the text is being called. This is a gidle routine. In other words, it's called on the gidle thread when there is nothing else to do in any of the other Orca threads. What we are seeing from the generated debug output is that there are several calls to the __idleHandler() routine the last of them being: gsf: __idleHandler called. _iH: context.startOffset: 191 _iH: offset: 41 _iH: calling progress callback: offset: 232 Each of these in turn call the __sayAllProgressCallback() routine in the gedit.py script. Therefore, the last offset given is 232 (the end of the + [2]1.1 Features line). Then the gedit __sayAllProgressCallback() routine is called again, this time because the "say all" operation has been interrupted. The last offset was 232, so the caret is positioned at that (wrong) point. The __idleHandler() routine in gnomespeechfactory.py is calling: (id, type, offset) = self.__eventQueue.get() each time to get the latest "say all" text caret offset. It looks like these events are put on the event queue in the notify() routine in the SpeechServer class in gnomespeechfactory.py: self.__eventQueue.put((id, type, offset)) This in turn is called from the notify() routine in the _Speaker class in gnomespeechfactory.py. This routine is called by GNOME Speech when the GNOME Speech driver generates a callback. So, in short, I think we are losing synchronisity between GNOME Speech and Orca here.
Created attachment 88341 [details] Test file from Halim, where he says it doesn't stop in the right place.
Created attachment 88342 [details] Orca run time output when debugging the interrupt problem.
Created attachment 88343 [details] [review] Various Orca print debug statement to help track down the interrupt problem.
Further evaluation by Will: > In the __speak() routine in gnomespeechfactory.py, there are calls like: > > text = self.__addVerbalizedPunctuation(text) > if orca_state.activeScript: > text = orca_state.activeScript.adjustForPronunciation(text) > > so what starts out as: > > Introduction > + [2]1.1 Features > * [3]2. > > ends up as the following text that is sent to the TTS: > > ` Introduction plus left bracket 2 right bracket 1 dot 1 Features > star left bracket 3 right bracket 2.` This is probably the problem. The index in the gnome-speech progress callback is an index into the string that was passed to gnome-speech. When the __idleHandler of gnomespeechfactory.py looks at the offset being passed to it, it is looking at the offset into the string that was passed to gnome-speech. It then mistakenly maps this to the original string, which we see from the above are two different strings: original: Introduction + [2]1.1 Features * [3]2. passed to gnome-speech: Introduction plus left bracket 2 right bracket... So...it seems as though there might need to be some sort of reverse mapping to handle the mapping of a character index in the string passed to gnome-speech to the index of a character index into the original text. Not quite sure of a way to do this -- hope you can come up with a good idea.
Created attachment 91041 [details] [review] Patch to hopefully fix this bug.
I've just given this patch some testing in gedit and OOWriter and am stopping where I would expect. I read documents with quite a bit of punctuation and still seemed to stop where I would expect.
Thanks. Patch committed. Setting bug state to "[pending]".
I have a question about the patch. The big 'for' loop that starts around line 707 sets newText in several spots. I'm not sure the updating of the self.textCharIndices is being done each time. For example, when expanding character names (newText += chnames.getCharacterName(oldText[i])), I don't see the indices being updated. Maybe the updating could be done in one spot at the end of the loop? for i in range(0, len(oldText)): try: ... except: ... self.textCharIndices.append(len(newText))
Alternatively, maybe self.text.CharIndices should represent where the character starts in the new string versus ends in the new string. So...maybe moving the append to the beginning of the loop would be better? for i in range(0, len(oldText)): self.textCharIndices.append(len(newText)) try: ... except: ...
Created attachment 91499 [details] [review] Revised patch based on feedback from Will. Nice. Thanks. Adjusted patch based on your suggestion from comment #9. Not committed yet, but it seems to work nicely. At least with the Halim QEMU test file.
> Nice. Thanks. Adjusted patch based on your suggestion from comment #9. > Not committed yet, but it seems to work nicely. At least with the Halim > QEMU test file. Looks good to me. :-) I say commit. Thanks!
Committed. Setting into "[pending]" state.
This seems to be working nicely. Closing as FIXED.