Bug 129403

Summary: speechSynthesis.speak of a zero length utterance kills future speech
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: 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 Flags
patch none

Description chris fleizach 2014-02-26 16:01:51 PST
Steps to Reproduce:
<script>
speechSynthesis.speak( new SpeechSynthesisUtterance(""));
speechSynthesis.speak( new SpeechSynthesisUtterance("hello"))
</script>

Expected Results:
It should say hello.

Actual Results:
speechSynthesis.speak is hung until I call speechSynthesis.cancel()

<rdar://problem/16168664>
Comment 1 chris fleizach 2014-02-26 16:51:04 PST
Created attachment 225322 [details]
patch
Comment 2 Mario Sanchez Prada 2014-02-27 02:21:44 PST
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?
Comment 3 chris fleizach 2014-02-27 09:11:50 PST
(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 4 WebKit Commit Bot 2014-02-27 09:44:14 PST
Comment on attachment 225322 [details]
patch

Clearing flags on attachment: 225322

Committed r164807: <http://trac.webkit.org/changeset/164807>
Comment 5 WebKit Commit Bot 2014-02-27 09:44:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Darin Adler 2014-02-27 12:15:14 PST
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?
Comment 7 chris fleizach 2014-02-27 12:18:09 PST
(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.