Summary: | speechSynthesis.speak of a zero length utterance kills future speech | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||
Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, mario, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
chris fleizach
2014-02-26 16:01:51 PST
Created attachment 225322 [details]
patch
Comment on attachment 225322 [details]
patch
Looks good to me, hence the r+. Still, I wonder that the problem this is fixing happens only for empty (zero-length) strings, or for anything that should not spoken at all, such as a string containing only blanks or unpronounceable characters. If that's the case, maybe it would make sense to check that in advance and early return if needed.
What do you think?
(In reply to comment #2) > (From update of attachment 225322 [details]) > Looks good to me, hence the r+. Still, I wonder that the problem this is fixing happens only for empty (zero-length) strings, or for anything that should not spoken at all, such as a string containing only blanks or unpronounceable characters. If that's the case, maybe it would make sense to check that in advance and early return if needed. > > What do you think? This a bit of an unknown unfortunately. I don't think an empty string would cause every synthesizer to choke for example. It just happens that it does so on Mac. This is the first case I've heard of it happening, so I'm happy to be conservative about what we strip out. Likewise, it's not always clear what a synthesizer can speak or not. Not all of them have API that allows you to know if a character is speakable or not. And even if something was not speakable, the synth may use that for pausing. Likewise, it may be able to normalize a character like é to e, even though it can't speak é. Comment on attachment 225322 [details] patch Clearing flags on attachment: 225322 Committed r164807: <http://trac.webkit.org/changeset/164807> All reviewed patches have been landed. Closing bug. Comment on attachment 225322 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=225322&action=review > Source/WebCore/Modules/speech/SpeechSynthesis.cpp:115 > + // Zero lengthed strings should immediately notify that the event is complete. > + if (utterance->text().isEmpty()) { > + handleSpeakingCompleted(utterance, false); > + return; > + } Are there any other strings that don’t work right? I guess that putting this check here relieves all the platforms of the need to handle empty strings, but what about strings with just spaces in them? Do those cause the same problems? (In reply to comment #6) > (From update of attachment 225322 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225322&action=review > > > Source/WebCore/Modules/speech/SpeechSynthesis.cpp:115 > > + // Zero lengthed strings should immediately notify that the event is complete. > > + if (utterance->text().isEmpty()) { > > + handleSpeakingCompleted(utterance, false); > > + return; > > + } > > Are there any other strings that don’t work right? I guess that putting this check here relieves all the platforms of the need to handle empty strings, but what about strings with just spaces in them? Do those cause the same problems? I haven't noticed issues with any strings with content in them. It's always possible there are such sequences, but I'm not aware of them. It's possible that synthesizers use such strings to help with prosity perhaps. |