added charLength to SpeechSynthesisEvent #50

sjdallst · 2019-03-07T21:04:30Z

There was already a PR about this same concept, but it seems to have become inactive. Creating this PR to get things moving again. Here is the original discussion. This change has picked up interest from both Microsoft and Google, so now seems like a good time to make the change.

Preview | Diff

sjdallst · 2019-03-07T22:21:24Z

Here's the corresponding test PR

travisleithead · 2019-03-13T17:51:14Z

Pinging @foolip, and maybe @rniwa or @marcoscaceres could find an appropriate reviewer from Apple/Mozilla? (Not sure who that might be.)

foolip · 2019-03-13T19:47:33Z

index.bs

+  <dt><dfn attribute for=SpeechSynthesisEvent>charLength</dfn> attribute</dt>
+  <dd>This attribute indicates the length of the text (word or sentence) that will be spoken corresponding to this event.
+  This attribute is the length, in characters, starting from this event's {{charIndex}}.
+  The user agent must return this value if the speech synthesis engine supports it or the user agent can otherwise determine it, otherwise the user agent must return undefined.</dd>


The problem of returning undefined mentioned in #30 (comment) remains to be solved here. If you're also planning to implement this (in Chromium?) we can just do what the implementation will do. There are really only two options:

Don't expose the charLength attribute if it's not supported, so that 'charLength' in SpeechSynthesisEvent.prototype is false (makes feature detection possible)

If this is a dynamic and depends on the speech synthesis engine at runtime, make the attribute nullable and say null here instead of undefined. There's some precedence for this in WebRTC.

I agree that nullable is the way to go, since it ends up being dynamic based on which speech synthesis engine you are using. Should this logic apply to charIndex, elapsedTime, and name as well? The current implementations seem to return the default values for these fields instead of undefined, and it might cause some compat issues if this is changed.

Do you mean that it returns 0? If making the distinction between zero and unknown isn't important to clients of the API we could change the spec to do the same. Bull null is fine too.

Would you be up to sending a PR to change the other attributes to change in a similar way?

foolip · 2019-03-13T19:49:37Z

@sjdallst @travisleithead thanks for sending this PR! If either of you plan to contribute more to this spec and implementation I'd be happy to give you write access so you can review each others PRs instead of being blocked on me.

index.bs

marcoscaceres · 2019-03-14T09:18:21Z

@andrenatal from the Mozilla side, perhaps?

marcoscaceres · 2019-03-14T09:20:29Z

index.bs

@@ -598,13 +598,15 @@ interface SpeechSynthesisUtterance : EventTarget {
 interface SpeechSynthesisEvent : Event {
    readonly attribute SpeechSynthesisUtterance utterance;
    readonly attribute unsigned long charIndex;
+    readonly attribute unsigned long? charLength;


why null instead of just zero?

This is related to the "return undefined" bits in https://w3c.github.io/speech-api/#speechsynthesisevent-attributes which don't actually make sense, it's not possible to return undefined through Web IDL. Using elapsedTime zero, name empty string would be fine, and charLength zero too. It's only for charIndex that zero may be ambiguous with a real value.

However, with charLength added, it would be possible to distinguish this case by checking if charLength is zero. @sjdallst do you think that would be OK? The existing Chromium implementation just returns the default values, right?

Argh, the above doesn’t sound amazing if it’s going to require special IDL treatment. Maybe we need a more in-depth spec review?

It won't be hard to find other things that aren't quite right or not precise enough, but this "return undefined" problem is quite confined, it doesn't interact with other problems in the spec AFAICT.

I think that the current state of things is a little ambiguous. If the user agent neglects charIndex completely, the client will see "undefined". If I get a 0 for charIndex, is this because the word or sentence is at the beginning of the text, or is it because the speech synthesizer does not support the charIndex? Returning null would remove some of this ambiguity, so a check for undefined or null would unambiguously tell the client whether this feature is supported, thus allowing for feature checking. Assuming the user agent follows the spec, this would also tell the client who is at fault (user agent or speech synthesis engine)

The current norm is the ambiguous option. I don't know if clients actually code for this ambiguous case, do platform checking, or hope for the best and let themselves break when features aren't supported, but following this norm would probably allow for the easiest and quickest implementation across browsers, and would let us resolve the undefined issue. However, issues might come later when clients have to keep track of what combination of browsers and platforms support these features.

The problem arises when the user agent (chromium, etc) supports the feature, but the speech synthesizer does not. For example, Windows, Mac, and, I think, Android have clear implementations for character length from speech events, but Linux (as far as I know) does not. Therefore, it makes sense to implement this feature in Chromium because it is supported by many speech synthesis platforms, but because the Linux version of chromium cannot get the value, it will be stuck returning a default value.

In order to do the prototype chain checking, I think a web platform like chromium would have to keep different versions of the SpeechSynthesisEvent for different platforms depending on whether or not that platform's speech synthesizer supports the attribute. This sounds like it would add a lot of unecessary code for any browser implementer that wanted to do this.

I think that if we just look at charLength, it's not a bad idea to return the default value. Word and sentence events are only fired when a word or sentence is about to spoken, and there aren't any speakable words of 0 characters.

I'm not as well versed in contributing to open source, but maybe charIndex is something that should be looked at in another PR? (yet could still be edited to reflect the current norm of returning a default value)

To @foolip 's point earlier. I think there would still be some slight ambiguity there. If I get an event with 0 charIndex and 0 charLength, the browser definitely doesn't support charLength, but whether it supports charIndex is still unclear. You would have to keep track of multiple events to see if charIndex is supported.

Linux version of chromium cannot get the value, it will be stuck returning a default value.

What's preventing this getting fixed on Linux tho? It seems like we are working around an OS limitation that could be fixed at the OS level at any time in the future.

(also noting that null adds further fingerprinting surface because it reveals something unique about the OS's capabilities - or lack of... not a huge thing, but it's a thing additional to consider)

While I'm leaning towards not nullable because I suspect it aligns with existing implementations, note that if we did make it nullable and then all OS limitations were resolved, it would just mean that null is never returned and we can eventually make it non-nullable again.

Ok it seems to me that we're leaning towards not nullable. I'll go ahead and make the change; along with edits to the descriptions of the other attributes.

Co-Authored-By: sjdallst <sjdallst@gmail.com>

foolip · 2019-03-14T17:08:25Z

@sjdallst, what are your thoughts on the overall "return undefined" situation? Do you think it's OK to just return the default values instead, or should we go with null everywhere?

foolip · 2019-03-15T10:06:16Z

@sjdallst can you summarize what browsers currently do on different platforms if the API is supported but the synthesizer turns out not to at runtime?

sjdallst · 2019-03-15T17:23:57Z

@sjdallst can you summarize what browsers currently do on different platforms if the API is supported but the synthesizer turns out not to at runtime?

Yeah, right now default values from SpeechSynthesisEventInit are being used, (so 0 for charIndex, etc)

sjdallst · 2019-03-15T17:29:44Z

@sjdallst, what are your thoughts on the overall "return undefined" situation? Do you think it's OK to just return the default values instead, or should we go with null everywhere?

I think the discussion here has led us towards doing default values. Nullable goes against current practices, and would eventually be phased out once a browser supports the full SpeechSynthesisEvent on all platforms.

foolip · 2019-03-15T22:58:24Z

Thanks for working through this and fixing the other attributes, @sjdallst!

Follows WICG/speech-api#50.

…hSynthesisEvent-constructor, a=testonly Automatic update from web-platform-tests Add test for charLength in SpeechSynthesisEvent constructor (#15732) Follows WICG/speech-api#50. -- wpt-commits: 3531dc2a994d918914e4751b97807ce8d79bf4b7 wpt-pr: 15732

Follows WICG/speech-api#50.

…hSynthesisEvent-constructor, a=testonly Automatic update from web-platform-tests Add test for charLength in SpeechSynthesisEvent constructor (#15732) Follows WICG/speech-api#50. -- wpt-commits: 3531dc2a994d918914e4751b97807ce8d79bf4b7 wpt-pr: 15732 UltraBlame original commit: ca56ec92fb4cd73e332c4f393cb4414faddb9211

added required entries for adding charLength to SpeechSynthesisEvent

1e8804b

This was referenced Mar 7, 2019

SpeechSynthesisEvent does not support charLength #51

Closed

Added test for charLength field in SpeechSynthesisEvent-constructor web-platform-tests/wpt#15732

Merged

foolip mentioned this pull request Mar 13, 2019

Add SpeechSynthesisEvent charLength attribute #30

Closed

foolip reviewed Mar 13, 2019

View reviewed changes

reworded to make charLength nullable

f0364f1

foolip reviewed Mar 14, 2019

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres reviewed Mar 14, 2019

View reviewed changes

add default value of null for charLength

eac9c13

Co-Authored-By: sjdallst <sjdallst@gmail.com>

merge

1a78c87

GurpreetV approved these changes Mar 15, 2019

View reviewed changes

foolip approved these changes Mar 15, 2019

View reviewed changes

foolip merged commit 959c2ed into WICG:master Mar 15, 2019

foolip pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 15, 2019

Add test for charLength in SpeechSynthesisEvent constructor (#15732)

3531dc2

Follows WICG/speech-api#50.

marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 23, 2019

Add test for charLength in SpeechSynthesisEvent constructor (#15732)

e05e3b1

Follows WICG/speech-api#50.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added charLength to SpeechSynthesisEvent #50

added charLength to SpeechSynthesisEvent #50

sjdallst commented Mar 7, 2019 •

edited by pr-preview bot

Loading

sjdallst commented Mar 7, 2019

travisleithead commented Mar 13, 2019

foolip Mar 13, 2019

sjdallst Mar 13, 2019

foolip Mar 14, 2019

foolip commented Mar 13, 2019

marcoscaceres commented Mar 14, 2019

marcoscaceres Mar 14, 2019

foolip Mar 14, 2019

marcoscaceres Mar 14, 2019

foolip Mar 14, 2019

sjdallst Mar 14, 2019

sjdallst Mar 14, 2019 •

edited

Loading

marcoscaceres Mar 15, 2019

marcoscaceres Mar 15, 2019

foolip Mar 15, 2019

sjdallst Mar 15, 2019

foolip commented Mar 14, 2019

foolip commented Mar 15, 2019

sjdallst commented Mar 15, 2019

sjdallst commented Mar 15, 2019

foolip commented Mar 15, 2019

added charLength to SpeechSynthesisEvent #50

added charLength to SpeechSynthesisEvent #50

Conversation

sjdallst commented Mar 7, 2019 • edited by pr-preview bot Loading

sjdallst commented Mar 7, 2019

travisleithead commented Mar 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foolip commented Mar 13, 2019

marcoscaceres commented Mar 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjdallst Mar 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

foolip commented Mar 14, 2019

foolip commented Mar 15, 2019

sjdallst commented Mar 15, 2019

sjdallst commented Mar 15, 2019

foolip commented Mar 15, 2019

sjdallst commented Mar 7, 2019 •

edited by pr-preview bot

Loading

sjdallst Mar 14, 2019 •

edited

Loading