After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 520494 - [verified] Keyboard review punctuation in Firefox
[verified] Keyboard review punctuation in Firefox
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: speech
2.21.x
Other All
: Normal normal
: 2.24.0
Assigned To: Willie Walker
Orca Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-03-05 11:10 UTC by Tomas Cerha
Modified: 2009-03-10 00:04 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
Use speakCharacter() instaed of speak() to speak the current character. (1.35 KB, patch)
2008-03-05 11:15 UTC, Tomas Cerha
none Details | Review
Use speakCharacter() instaed of speak() to speak the current character. (1.94 KB, patch)
2008-03-05 15:46 UTC, Tomas Cerha
none Details | Review
Use speakCharacter() instead of speak() to speak the current character. (1.93 KB, patch)
2008-03-06 12:09 UTC, Peter Vágner
none Details | Review
Use speakCharacter() instead of speak() to speak the current character (revision 2). (3.92 KB, patch)
2008-03-19 11:22 UTC, Peter Vágner
none Details | Review
Slightly revised patch. (4.02 KB, patch)
2008-03-26 15:50 UTC, Rich Burridge
needs-work Details | Review
Revised patch - 27th March 2008 - SVN trunk. (3.67 KB, patch)
2008-03-27 16:31 UTC, Rich Burridge
none Details | Review
the change to gecko.py has been lost in the trunk branch of Orca (716 bytes, patch)
2008-04-29 08:04 UTC, Peter Vágner
none Details | Review
Patch for trunk (12.96 KB, patch)
2008-05-11 16:22 UTC, Willie Walker
none Details | Review
Patch to address missing "newline" in previous patch (13.45 KB, patch)
2008-05-12 14:05 UTC, Willie Walker
none Details | Review
Patch with all the whitespace diffs in it (98.67 KB, patch)
2008-05-12 15:11 UTC, Willie Walker
committed Details | Review
Patch to not normalize character name before sending to the speech server's speakCharacter (698 bytes, patch)
2008-05-19 18:44 UTC, Willie Walker
committed Details | Review

Description Tomas Cerha 2008-03-05 11:10:38 UTC
Please describe the problem:
Punctuation is not read while reviewing text by arrow keys in Firefox 3

Steps to reproduce:
1. Set the Speech System to Speech Dispatcher
2. Set the Orca punctuation level to anything except All
2. Open any web page in Firefox 3
3. Use arrow keys to review the text character by character


Actual results:
The punctuation characters, such as dots, commas, exclamation marks etc are not read.

Expected results:
The punctuation characters should always be read in the keyboard review regardless the current punctuation level.

Does this happen every time?
Yes

Other information:
The problem is in the Gecko script, which uses the `speak()' command, instead of `speakCharacter()'.  I'll submit a patch soon.  The problem doesn't reveal with Gnome Speech, since the punctuation verbalization is handled on a different level there.
Comment 1 Tomas Cerha 2008-03-05 11:15:45 UTC
Created attachment 106607 [details] [review]
Use speakCharacter() instaed of speak() to speak the current character.

The patch is against Orca trunk, but its a on-liner, so there is no problem to make one for the gnome-2-22 branch.
Comment 2 Willie Walker 2008-03-05 15:20:40 UTC
Note to Orca team from the Orca list:

"With the patch applyed Gecko stuff totally overrides Orca character processing therefore I am not sure at this point but it is good idea to test his patch with gnome speech.
Just note the difference how review by characters work in flat review, in gnome-terminal, in Gedit, in evolution etc and you will be able to understand what I am trying to explain."
Comment 3 Tomas Cerha 2008-03-05 15:46:35 UTC
Created attachment 106623 [details] [review]
Use speakCharacter() instaed of speak() to speak the current character.

This version does the same for the default script (in addition to the Gecko script).
Comment 4 Tomas Cerha 2008-03-05 16:00:38 UTC
I made the same change to the default script in the above patch.  Although it seems strange to omit the call to `chnames.getCharacterName(character)' I believe it works well, since the Gnome Speech backend always performs verbalization of the spoken text by calling `__addVerbalizedPunctuation()'.  Anyway, I recommend testing this with the Gnome Speech backend to avoid a regression.
Comment 5 Peter Vágner 2008-03-06 10:14:19 UTC
I have tryed this with gnome speech and it seems to be working fine.
Also I've switched to orca-2-22 branch and found out that src/orca/default.py is already patched. But still I see Orca's processing being performed while reading text in the gnome-terminal, in gedit in evolution and else where. Only Gecko (firefox 3 and thunderbird 3) documents are working fine and flat review is also working fine.
More ower I've tryed to search for some more occurences of the place where such a one-line fix might be applied and I am unable to find it at the moment.
I've only noticed Orca is reporting new line characters like this in the default.py more concrete in the 
sayWord(self, obj)
sayCharacter(self, obj)
methods.

               peech.speak(chnames.getCharacterName("\n"), voice, False)

This is also debatable because I don't know what is better let the synth to do this or let the it as is. Please note with gnome speech this processing may be performed twice in such places like this one.
Comment 6 Peter Vágner 2008-03-06 12:09:38 UTC
Created attachment 106676 [details] [review]
Use speakCharacter() instead of speak() to speak the current character. 

Converted the above patch for branch gnome-2-22. It fixes Gecko.py as well as default.py . Prewiously I was wrong saying 2-22 does not need patch for default.py.
Comment 7 Peter Vágner 2008-03-06 12:57:13 UTC
(In reply to comment #5)
> Also I've switched to orca-2-22 branch and found out that src/orca/default.py
> is already patched. But still I see Orca's processing being performed while
> reading text in the gnome-terminal, in gedit in evolution and else where. 

wrong. I did a mistake while applying the patch. This all is working fine. I think this should be considered fixed if anyone else is able to confirm it.
Comment 8 Willie Walker 2008-03-06 17:24:41 UTC
(In reply to comment #7)
> (In reply to comment #5)
> > Also I've switched to orca-2-22 branch and found out that src/orca/default.py
> > is already patched. But still I see Orca's processing being performed while
> > reading text in the gnome-terminal, in gedit in evolution and else where. 
> 
> wrong. I did a mistake while applying the patch. This all is working fine. I
> think this should be considered fixed if anyone else is able to confirm it.

Thanks so much for testing this.  I just want to be clear -- you're experiences are that both trunk and the gnome-2-22 branch give you the desired behavior after the last patch is applied.  Is that correct?
Comment 9 Peter Vágner 2008-03-06 17:58:08 UTC
(In reply to comment #8)
> Thanks so much for testing this.  I just want to be clear -- you're experiences
> are that both trunk and the gnome-2-22 branch give you the desired behavior
> after the last patch is applied.  Is that correct?
> 

yes I have tested with both the branches and both are working as I expect. No mather which synth backend is chosen. Of course gnome speech does things differently as speech-dispatcher but there is no regression all punctuation marks are spoken. The only difference is that with speech-dispatcher backend all the character verbalization is done by synthesizer and with gnome speech it's done in Orca or in gnome speech actually I am not sure because I don't know gnome speech good enough.
comment #5 in this report should be ignored because I've applied patch provided by Tomas to the branch 2-22 incorrectly. It's why I've converted it for branch 2-22.
Comment 10 Peter Vágner 2008-03-10 10:56:56 UTC
Please note this patch also fixes the very same issue in thunderbird 3. It is quite obvious since it's Gecko based but at least we know what else to test with if needed.
Comment 11 Willie Walker 2008-03-11 14:06:21 UTC
First coarse pass at GNOME 2.24 planning.
Comment 12 Willie Walker 2008-03-17 17:49:53 UTC
See also bug 523012.
Comment 13 Peter Vágner 2008-03-19 11:22:49 UTC
Created attachment 107596 [details] [review]
Use speakCharacter() instead of speak() to speak the current character (revision 2). 

Except of the prewious fixes this patch also fixes anouncement of deleted characters (by backspace or delete key) and it also fixes spelling (incl phonetic spelling) in default.py. Perhaps some problems will be found else where. It's for gnome-2-22 branch and no regression has been found so far. I have tested with speech-dispatcher as well as gnome-speech. Synth used is always Espeak with slovak, english and czech voices.
Comment 14 Willie Walker 2008-03-25 14:44:17 UTC
If this doesn't introduce any freeze break issues, we should also look to get this in for 2.22.x.
Comment 15 Peter Vágner 2008-03-26 14:43:29 UTC
Hello guys,
I am wondering perhaps tests for the lenght of a string should also be made at some more places e.g. valueChangeEvent. Try to do the following
1) open nautilus
2) hit ctrl+l and box where you can type the patch to open comes up
3) enter /home and hit the end key. /home/ gets automatically completed to /home/ . When that slash is supposed to be spoken speak is used instead of speakCharacter and it's not read.
What do you all think?
I think I can extend the patch above I would just like to get someone's opinion first.
I was also thinking that an easy thing would be to do the test in the speak method but that would force all strings of length 1 to be spoken by speakCharacter. I am not sure we want this because in some languages there might be words consisting of single letter only.
Comment 16 Rich Burridge 2008-03-26 15:50:46 UTC
Created attachment 108064 [details] [review]
Slightly revised patch.

Revised patch pylints to 10.0. Committed to SVN trunk and gnome-2-22 branch.
Comment 17 Rich Burridge 2008-03-26 15:51:50 UTC
Thanks Peter and Tomas. Moving to "[pending]".

Mike, please test.
Comment 18 Mike Pedersen 2008-03-26 21:20:02 UTC
If Peter and Tomas are happy hear so am I.  
Comment 19 Tomas Cerha 2008-03-27 07:56:17 UTC
(In reply to comment #18)
> If Peter and Tomas are happy hear so am I.  

I'm sorry I don't have a chance to test this now.  Just from looking at the patch I am a little worried.  As Peter correctly notes, it would be really wrong to send a string to speakCharacter() instead of speak() just because it happens to have the length of 1.  speakCharacter() should only be used in a context where we are absolutely sure that we are spelling or reading something by character.  It is definitely not desirable to send a one-character word to the synth as speakCharacter() and speak() is perfectly appropriate in this case.  Thus I would only be for applying the additional changes if we can be absolutely sure that we can't miss-interpret a one character word in given context.  If there is any chance we could, I'd rather be for applying the original patch without these additions and solving them separately.

There is also another quite minor problem, but as this problem repeats very often in Orca, I'd like to emphasize the importance of this.

Applying the function `len()' to an UTF-8 encoded string gives no usefull information.  It returns  the number of bytes the string takes in the UTF-8 encoding, which is not what we want.

All Python string functions and methods must be used with Python unicode strings to produce reasonable results.  The only real solution would be to switch Orca completely to using Python unicode strings (this is how the Python unicode support is meant to be used and when it is very practical).  Until then, however, we must decode the string before applying any function or method on it.  Thus len(string.decode('utf-8')) will return the real number of characters in the string.
Comment 20 Tomas Cerha 2008-03-27 08:19:28 UTC
One more thought on the previous issue.  It would make sense to me to assume, that when a one character string is sent to a synth, it should be spoken regardless the current punctuation level.  As far as I know, many synths actually do that.  In this case, Orca would not be the right place to fix the problem and we should probably test the behavior with different synths and as well with Gnome Speech and consult the problem with synth engine developers.
Comment 21 Peter Vágner 2008-03-27 12:06:18 UTC
Well the parts I tested the length of character and send 1 character long strings to the speakCharacter should be all right according to my tests. I haven't included further such modifications because of the things I am not sure about -- which I have mentioned in my yesterday's comment. Only the issue which might really cause problems is utf-8 strings examination. I have to take a look into it to ensure this is all right. 
Comment 22 Rich Burridge 2008-03-27 16:16:18 UTC
I've backed out the last patch from the gnome-2-22 branch and SVN 
trunk and marked it as "needs-work". I also just spoke with Will, 
and we think we know what needs to be done to remedy this.

To that end, I plan to just attach a new patch to this bug (rather
than check it in), and get people to test it first before I commit
it.

Hopefully that will be in the next hour or so.

Thanks.
Comment 23 Rich Burridge 2008-03-27 16:31:19 UTC
Created attachment 108118 [details] [review]
Revised patch - 27th March 2008 - SVN trunk.

Here's a further revised patch (against SVN trunk) that adds the
following changes over the previous patch:

1/ Does len(character.decode('utf-8')) instead of len(character)
2/ Calls chnames.getCharacterName(character) in the speakCharacter()
   method in gnomespeechfactory.py.

Patch not committed yet. Please test and let me know of any problems
you find.

Thanks!

PS: Although I haven't tried it, hopefully this patch should also 
    work against Orca in the gnome-2-22 branch with slight line 
    offset differences (I took out the ChangeLog part of the diffs 
    for now).
Comment 24 Willie Walker 2008-03-28 14:31:07 UTC
From a visual inspection, this looks like what we talked about yesterday (thanks!).  I'd feel comfortable if Peter and other communities members were to give it more testing.  If people run into problems, I'd also like to know more detail about what speech system they are using (gnome-speech, speech dispatcher) and if they could temporarily switch from one to the other for more testing coverage.
Comment 25 Mesar Hameed 2008-03-28 23:42:23 UTC
I assume the following html is reasonable as a test?


<html><head>
<title>punctuation test</title>
</head><body>
<p>A line before the tests.</p>
<p>.!"|@~$%^&*()_+=\'/,>?<#{}[]</p>
</body></html>

if true then:
using orca 3772 sd 0.6.6, 

sd+ibmtts (with and without patch) and with all possible punctuation levels:
all characters are spoken when arrowing/flat review arrowing.
this patch doesnt do anything for IBMTTS.

sd+espeak:
independant of orca punc level
without patch, not all chars are spoken when left/right and flat review left/right.
with patch:
all spoken with left/right and for flatleft/right.


seperate issue:

ibmtts: infact, the punctuation setting does not seem to have any effect at all on the characters being spoken when we use line nav. i.e if i stand on:
<p>A line before the tests.</p>
change the punctuation setting, and go back to ff3, and arrow down.
exactly the same is being spoken.
The oddety is that it isnt actually reading all chars at any point.
but the same chars are being read every time.


sd with espeak and line nav:
(all punct settings accept the "all" setting)
we seem to say the same when we arrive on the line. for all except the "all" punct level.
when "all" is set, then evrything is spoken.


sorry cant test gnome-speech, it doesnt seem to be working at all since speech-dispatcher was installed.

Hope this is useful

Comment 26 Mesar Hameed 2008-03-29 00:38:57 UTC
sorry forgot, all the above was done inside ff3.
I believe the situation is not the same elsewhere.
i guess punct level doesnt matter when arrowing now?
anyway, its set to most.

following is for gedit
text:
<property name="visible">True</property>

sd+ibmtts arrowing:
"(scilance) property (silance) name equals quote visible quote (silance) True (scilance) slash property (scilance)"

sd+espeak arrowing:
"l angle property space name equals quotes visible quotes r angle true l angle stroke property r angle"
Comment 27 Joanmarie Diggs (IRC: joanie) 2008-03-29 18:52:28 UTC
In FF3, for gnome-speech using Swift and eSpeak with punctuation level of some:

* When moving left and right, all characters are spoken, with and 
  without the patch.

* When moving by line, the characters that are spoken by each synth
  is the same for that synth, with and without the patch.

* When moving by line, each synth says something a bit different:

  Swift speaks: .!|@~$%^&*_\/>?<#}
  Swift AWOL:   "()',{[]

  eSpeak speaks: .@~$%^&*+=\/#
  eSpeak AWOL:   !"|()_',>?<{}[]
Comment 28 Rich Burridge 2008-03-29 19:37:44 UTC
Comment from Jan Buchal to the Orca mailing list:

    RB> Steps to reproduce: 1. Set the Speech System to Speech
    RB> Dispatcher 2. Set the Orca punctuation level to anything except
    RB> All 2. Open any web page in Firefox 3 3. Use arrow keys to
    RB> review the text character by character

    RB> Actual results: The punctuation characters, such as dots,
    RB> commas, exclamation marks etc are not read.
Are read in my orca settings. I have last svn snapshot of orca and I use
speech dispatcher with festival output. So it works without your patch.

With espeak output not works. It works with your patch only.
Comment 29 Rich Burridge 2008-03-29 19:40:49 UTC
Comment from Peter Vagner to the Orca mailing list, with reference
to the previous comment:

"Huh guys I am now totaly confused or lost or something. I simply don't 
know what to say. With the patch it works well for me. I am using Espeak 
through speech-dispatcher. I have also tested with gnome speech and I 
think expanding characters to their names is not necessary in 
gnomespeechfactory because that was always working great.

After reading this comment by Mr. Buchal I am afraid the problem might 
be somewhere in the speech-dispatcher, espeak or festival or something 
else than Orca. Or festival simply treats all strings with length of 
1 as characters and does the correction itself where Orca fails.

With increasing number of comments posted to the bugtracker this reveals 
too many issues I haven't realised yet. I will start comparing it on 
Monday as I am using Ubuntu regularly during week-days only.

Thanks for all the comments"
Comment 30 hanke 2008-03-31 10:13:04 UTC
Hello, I'm sorry, but I think the idea of the later part of the patch (comparation of message length to one character) is really very wrong.

In many languages, there are ordinary one character words (prepositions, articles etc.). Now each time there happens to be only one such a word in a given area, this word will be spoken as a character (possibly with different voice, very likely with different speed), there will be a different importance priority for it etc? This is not right.

Also, consider that in many languages (like Chinese), there are really many words (all kinds of words, not just prepositions, but substantives, even with quite long pronounciation) being represented by a single UTF-8 character...

The sayCharacter event should really be reserved to cases where the message can be positively identified as a single character event while cursor-navigating though text, while deleting text by characters and in other cases, where it is desirable to treat the message as a "character event" as opposed to "part of ordinary text". This decision must always be based on the context known to the synthesizer (result of an action), not on the form of the text. If this is only an attempt to "guess", then there would be no reason for the patch at all, since a guess without application context can as well be done in the synthesizer itself.

I strongly recommend returning to the latest patch proposed by Tomas Cerha. The above very simple test case might not reveal the problems brought, but it will very likely result in bugs later on.

Comment 31 Peter Vágner 2008-03-31 11:55:29 UTC
(In reply to comment #30)
> The sayCharacter event should really be reserved to cases where the message can
> be positively identified as a single character event while cursor-navigating
> though text, while deleting text by characters and in other cases, where it is
> desirable to treat the message as a "character event" as opposed to "part of
> ordinary text". This decision must always be based on the context known to the
> synthesizer (result of an action), not on the form of the text. If this is only
> an attempt to "guess", then there would be no reason for the patch at all,
> since a guess without application context can as well be done in the
> synthesizer itself.
> 
That's exactly what I am trying to do. Ofcourse there might still be problems but currently in orca there is the same event fired when deleting a word as well as when deleting a character. How shal we identify if it's a word of if it's a character?
Also second place where such comparison is made is spelling. When punctuation is set to None and synth gets a punctuation mark it just speaks nothing so the only way to fix this is to use speakCharacter. Perhaps anyone is able to think of better condition which may elliminate as many issues as possible but I am afraid there's not very much we can do at this point.
In my opinion the places where this is compared we are really dealing with single characters but again I may be mistaken I don't know many cultures to ensure.
Comment 32 Rich Burridge 2008-03-31 21:35:39 UTC
Thanks for all the testing and comments on this one.

It seems we have a difference of opinion on how this
should work, so I think it's time for Mike and Will to
clearly define what Orca should be doing here.


Comment 33 Willie Walker 2008-04-01 14:02:03 UTC
I'm not sure there is a disagreement.  Judging from Hynek's comment -- "Now each time there happens to be only one such a word in a given area, this word will be spoken as a character" -- I think there's just a misunderstanding of what the code is doing.

> The sayCharacter event should really be reserved to cases where the 
> message can be positively identified as a single character event while 
> cursor-navigating though text, while deleting text by characters and 
> in other cases, where it is desirable to treat the message as a 
> "character event" as opposed to "part of ordinary text".

Agreed.  Let's analyze the use of each speakCharacter after the patch has been applied.

[GOOD] Gecko.py:sayCharacter:  used in here to speak a character as a result of an explicit user action to read a character.

[GOOD] default.py:sayCharacter:  used in here to speak a character as a result of an explicit user action to read a character.

[GOOD] default.py:onTextDeleted: used in here to speak a character if it is determined that only 1 character has been deleted.  I believe this is correctly treating it as a "character event" as opposed to "part of ordinary text".

[GOOD] default.py:spellCurrentItem: speaks the characters of a word one character at a time.

[GOOD] default.py:_reviewCharacter: speaks the character because that's what we want to do.

[GOOD] default.py:phoneticSpellCurrentItem: speaks the character if there is not a 'military' word for it.

Now...there's also other places where chnames is used:

[QUESTIONABLE] default.py:sayWord: expands out the "\n" character if we crossed a line boundary using speech.speak(chnames.getCharacterName("\n"), voice, False).  I say this is questionable because it probably should use the new behavior of speech.speakCharacter.

[QUESTIONABLE] default.py:sayCharacter: same as above.

[GOOD] default.py:_addRepeatSegment: speaks the number of times a character has been repeated.

[GOOD] keynames.py:getKeyName: used in the logic to get the name of a key for key echo.
Comment 34 Eitan Isaacson 2008-04-01 19:45:25 UTC
> All Python string functions and methods must be used with Python unicode
> strings to produce reasonable results.  The only real solution would be to
> switch Orca completely to using Python unicode strings (this is how the Python
> unicode support is meant to be used and when it is very practical).  Until
> then, however, we must decode the string before applying any function or method
> on it.  Thus len(string.decode('utf-8')) will return the real number of
> characters in the string.
> 

I hope we are on the way to having all strings being Unicode.
See bug #524740 and bug #524741.
Comment 35 Willie Walker 2008-04-01 20:48:59 UTC
We're going to pass on this patch for GNOME v2.22.1 and will wait until Tomas gets back to carry on the discussion more with a goal of addressing this for GNOME v2.22.2.  

In addition, if all looks fine, I think the safer thing to do would be to leave the "QUESTIONABLE" things as is since they already work, and they are mostly for giving the "newline" presentation when crossing line boundaries.
Comment 36 hanke 2008-04-02 07:08:00 UTC
Hello all, thanks for the explanation. Given that the comparision to character length of one is only made on text deletion (which was not obvious to me from the context included in the patch), I think it is correct. I therefore apologize for my comment.
Comment 37 Peter Vágner 2008-04-29 08:04:44 UTC
Created attachment 110086 [details] [review]
the change to gecko.py has been lost in the trunk branch of Orca

As the description says. I have just updated to Orca 2.23.1 and when using firefox or thunderbird spaces and punctuation marks are no longer read when Orca is speaking through speech-dispatcher
Comment 38 Willie Walker 2008-04-29 13:49:12 UTC
(In reply to comment #37)
> Created an attachment (id=110086) [edit]
> the change to gecko.py has been lost in the trunk branch of Orca
> 
> As the description says. I have just updated to Orca 2.23.1 and when using
> firefox or thunderbird spaces and punctuation marks are no longer read when
> Orca is speaking through speech-dispatcher
> 

Thanks Peter!  I think we may need to do a thorough audit of where Orca is performing speak operations and make sure the appropriate method is called.  I see this on trunk, meaning there are 339 speech.speak[Utterances,Character,KeyEvent] operations in 32 files:

bash-3.2$ find . -name \*.py | xargs grep "speech[.]speak" | wc -l
339

bash-3.2$ find . -name \*.py | xargs grep "speech[.]speak" | cut -f1 -d: | sort -u | wc -l
32

Is this an audit you'd be willing to perform?
Comment 39 Willie Walker 2008-05-11 16:22:00 UTC
Created attachment 110719 [details] [review]
Patch for trunk

This patch reflects an audit of all the calls to the various speech.speak* methods, and attempts to make sure the appropriate method is used and that decision logic for things like determining the names of characters is pushed into the speech factory.  It pylints fine, but there are some regressions in the gtk-demo tests that I need to investigate:

1) The new behavior of this one (i.e., speaking 'space' instead of ' ') might actually be a fix:

Test 3 of 10 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline.py:Arrow to end of 'This'
DIFFERENCES FOUND:
  BRAILLE LINE:  'gtk-demo Application Application Window Frame ScrollPane This is a test. $l'
       VISIBLE:  'This is a test. $l', cursor=2
  BRAILLE LINE:  'gtk-demo Application Application Window Frame ScrollPane This is a test. $l'
       VISIBLE:  'This is a test. $l', cursor=3
  BRAILLE LINE:  'gtk-demo Application Application Window Frame ScrollPane This is a test. $l'
       VISIBLE:  'This is a test. $l', cursor=4
  BRAILLE LINE:  'gtk-demo Application Application Window Frame ScrollPane This is a test. $l'
       VISIBLE:  'This is a test. $l', cursor=5
  SPEECH OUTPUT: 'h'
  SPEECH OUTPUT: 'i'
  SPEECH OUTPUT: 's'
- SPEECH OUTPUT: 'space'
?                 ^^^^^

+ SPEECH OUTPUT: ' '
?                 ^

2) This one may also be a fix (speaking 'dot' instead of '.'):

Test 1 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:Left once from end to '.' after 'wild'
DIFFERENCES FOUND:
  BRAILLE LINE:  'in the wild. $l'
       VISIBLE:  'in the wild. $l', cursor=12
- SPEECH OUTPUT: 'dot'
?                 ^^^

+ SPEECH OUTPUT: '.'
?    

3) This one is a definite regression (speaking '\n' instead of 'newline'), and there's a lot them:

Test 16 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:Ctrl+Right to end of 'This'
DIFFERENCES FOUND:
  BRAILLE LINE:  'gtk-demo Application Application Window Frame ScrollPane This is a test.  $l'
       VISIBLE:  'This is a test.  $l', cursor=5
- SPEECH OUTPUT: 'newline'
?                 --------

+ SPEECH OUTPUT: '
+ '
  SPEECH OUTPUT: 'This '

I also need to test more with at least OOo and Gecko and make sure the testing coverage covers the code in question.  This is going to take a while and we won't get it in for 2.23.2.
Comment 40 Willie Walker 2008-05-12 14:05:01 UTC
Created attachment 110764 [details] [review]
Patch to address missing "newline" in previous patch

It turns out that the missing 'newline' regression was a side effect of using the 'log only' form (i.e., no real speech output) of speech.  By adjusting speech.py:speakCharacter to use chnames, the regression is gone and 'newline' is output as it was before.

This leaves the remaining diffs for the gtk-demo regression tests, which I believe reflect the fix for this bug (i.e., they are good diffs, not bad diffs).  I'll talk about these with Mike today.  If they are OK, then I'll look at the Firefox and OOo tests to see the impact there.

Test 30 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_2 to flat review ' '
DIFFERENCES FOUND:
  BRAILLE LINE:  '  $l'
       VISIBLE:  '  $l', cursor=1
- SPEECH OUTPUT: ' '
?                 ^

+ SPEECH OUTPUT: 'space'
?                 ^^^^^


Test 42 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:Ctrl+Delete right 'is'
DIFFERENCES FOUND:
  BRAILLE LINE:  'gtk-demo Application Application Window Frame ScrollPane  is a test.  $l'
       VISIBLE:  ' is a test.  $l', cursor=1
- SPEECH OUTPUT: ' '
?                 ^

+ SPEECH OUTPUT: 'space'
?                 ^^^^^

Test 45 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:BackSpace '.' after 'test'
DIFFERENCES FOUND:
  BRAILLE LINE:  'This is only a test. $l'
       VISIBLE:  'This is only a test. $l', cursor=21
  BRAILLE LINE:  'This is only a test. $l'
       VISIBLE:  'This is only a test. $l', cursor=20
- SPEECH OUTPUT: '.'
?                 ^

+ SPEECH OUTPUT: 'dot'
?                 ^^^


Test 55 of 87 SUCCEEDED: /export/home/wwalker/Desktop/orca/trunk-speak/test/kTest 58 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_1 to flat review space
DIFFERENCES FOUND:
  BRAILLE LINE:  'This is only  $l'
       VISIBLE:  'This is only  $l', cursor=5
- SPEECH OUTPUT: ' '
?                 ^

+ SPEECH OUTPUT: 'space'
?                 ^^^^^


Test 60 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:KP_3 to flat review space
DIFFERENCES FOUND:
  BRAILLE LINE:  'This is only  $l'
       VISIBLE:  'This is only  $l', cursor=5
- SPEECH OUTPUT: ' '
?                 ^

+ SPEECH OUTPUT: 'space'
?                 ^^^^^


Test 81 of 87 FAILED: /export/home/wwalker/Desktop/orca/trunk-speak/test/keystrokes/gtk-demo/role_text_multiline_navigation.py:Insert+KP_1 to flat review end of line
DIFFERENCES FOUND:
  BRAILLE LINE:  'Open & y toggle button Quit panel GTK! $l'
       VISIBLE:  'l GTK! $l', cursor=6
- SPEECH OUTPUT: '!'
?                 ^

+ SPEECH OUTPUT: 'exclaim'
?                 ^^^^^^^
Comment 41 Willie Walker 2008-05-12 15:11:38 UTC
Created attachment 110773 [details] [review]
Patch with all the whitespace diffs in it

This patch includes a diff with all the whitespace in it.  I had made the previous patches using "svn diff --diff-cmd diff -x -uw", but Rich noticed that it seemed to create a diff with bad indentation in it.
Comment 42 Willie Walker 2008-05-13 13:13:25 UTC
I tested this patch with gedit, gtk-demo, OOo, and Firefox 3, and it seems to be doing the right things when using gnome-speech.  I also talked with Mike about the new verbalization of things like 'space' for ' ', 'exclaim' for '!', etc., and we agreed this is the desired behavior and part of the fix for this bug.

I'm going to mark this as testing required for now since I'm still a little shy about the overall changes being made.  In addition, I didn't test this with speech dispatcher, but I suspect some changes might be needed in there since this patch pushes character verbalization logic down to the speech factory.  In looking at speech dispatcher briefly, I see that this patch may actually cause the newline audio cue (as supported by speech dispatcher) to be played appropriately.
Comment 43 Willie Walker 2008-05-16 16:34:28 UTC
Patch committed.  Marking as [pending].  Note to self: UPDATE THE REGRESSION TESTS TO REFLECT THE NEW AND IMPROVED SPEECH OUTPUT.
Comment 44 Mike Pedersen 2008-05-16 16:44:07 UTC
Should I hold off for a bit on verifying this one until we see what feedback we get from speech-dispatcher users? 
Comment 45 Willie Walker 2008-05-16 16:48:31 UTC
(In reply to comment #44)
> Should I hold off for a bit on verifying this one until we see what feedback we
> get from speech-dispatcher users? 
> 

Yes, please hold off.  I just sent a notice to the orca-list warning them about this change.
Comment 46 Tomas Cerha 2008-05-19 06:21:18 UTC
Yes, there is a regression with the Speech Dispatcher backend.  As reported by users on the list, "Symbol 0" is pronounced instead of any character name in keyboard review.

This is caused by verbalizing the character name before calling speech server's speakCharacter() method in speech.speekCharacter.  The problem can be easilly fixed by removing the line "character = chnames.getCharacterName(character)" from this method.  This should not influence Gnome Speech, since the same operation is done there as well (you are actually calling it twice now with Gnome Speech).

I'm not sending a patch since I believe this one line change is clear and simple, but let me know if you need one.

I appreciate the changes made in the speech infrastructure and I believe it will clean the interface up for us to be able improve it further.  In general I am for moving a lot of the logic into the 'speech' module and the character verbalization is definitely one of them.  But since it is not directly related to this bug, I'd rather solve that separately and finish this bug now leaving the verbalization up to the speech server, which works correctly now.
Comment 47 Willie Walker 2008-05-19 18:44:31 UTC
Created attachment 111174 [details] [review]
Patch to not normalize character name before sending to the speech server's speakCharacter

Patch committed.  Pylint's, regression tests.  Should make speech dispatcher work again.
Comment 48 Mike Pedersen 2008-05-21 18:30:15 UTC
Seems like everyone is happy with this one now.