-
Notifications
You must be signed in to change notification settings - Fork 38
Replace SpeechRecognitionPhraseList with an ObservableArray #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
index.bs
Outdated
| attribute unsigned long maxAlternatives; | ||
| attribute boolean processLocally; | ||
| attribute SpeechRecognitionPhraseList phrases; | ||
| attribute sequence<SpeechRecognitionPhrase> phrases; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is invalid Web IDL; attributes cannot be sequences.
ObservableArray is probably the correct choice here. See https://webidl.spec.whatwg.org/#idl-observable-array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh good call, I'll switch this to ObservableArray
|
On second thought, I'm questioning if this is the right thing to do here for the following reasons:
My suggestion is to keep the SpeechRecognitionPhraseList as-is, or alternatively use a FrozenArray it's crucial to avoid adding a new type. @domenic - What are your thoughts on this? |
|
It's indeed crucial to avoid adding new fake arrays to the web platform. That is a strongly deprecated practice. The question is whether you want this to be mutable, or only settable by the constructor. If it's supposed to be mutable, ObservableArray is the right choice. If it's only settable by the constructor, then adding a constructor option (that would be a Some recent discussion about this in whatwg/webidl#1495 (comment). |
|
I see, it sounds like ObservableArray would be the best choice here since it's mutable. Do you know why there's still only a single usage of the ObservableArray in Chromium despite being introduced 4 years ago? |
|
I think it's a matter of successive filtering. You'd need:
I think it's pretty reasonable to expect only one API to meet those filters over the course of 4 years. We don't create that many new APIs, period, as an absolute number. |
SHA: 4d6e11e Reason: push, by evanbliu Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…itionPhrase> This CL replaces SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase>. Spec changes: WebAudio/web-speech-api#169 Bug:381349238 Change-Id: I9cb4bdafd7c74cf48a4d7f3684af644df1eff4c0
…itionPhrase> This CL replaces SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase>. Spec changes: WebAudio/web-speech-api#169 Bug: 381349238 Change-Id: I9cb4bdafd7c74cf48a4d7f3684af644df1eff4c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6696798 Reviewed-by: Domenic Denicola <[email protected]> Commit-Queue: Evan Liu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1483461}
…itionPhrase> This CL replaces SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase>. Spec changes: WebAudio/web-speech-api#169 Bug: 381349238 Change-Id: I9cb4bdafd7c74cf48a4d7f3684af644df1eff4c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6696798 Reviewed-by: Domenic Denicola <[email protected]> Commit-Queue: Evan Liu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1483461}
…itionPhrase> This CL replaces SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase>. Spec changes: WebAudio/web-speech-api#169 Bug: 381349238 Change-Id: I9cb4bdafd7c74cf48a4d7f3684af644df1eff4c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6696798 Reviewed-by: Domenic Denicola <[email protected]> Commit-Queue: Evan Liu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1483461}
… ObservableArray<SpeechRecognitionPhrase>, a=testonly Automatic update from web-platform-tests Replace SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase> This CL replaces SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase>. Spec changes: WebAudio/web-speech-api#169 Bug: 381349238 Change-Id: I9cb4bdafd7c74cf48a4d7f3684af644df1eff4c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6696798 Reviewed-by: Domenic Denicola <[email protected]> Commit-Queue: Evan Liu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1483461} -- wpt-commits: 82b74727278342807e9b42c9f29ecc7bdddc0495 wpt-pr: 53634
… ObservableArray<SpeechRecognitionPhrase>, a=testonly Automatic update from web-platform-tests Replace SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase> This CL replaces SpeechRecognitionPhraseList with ObservableArray<SpeechRecognitionPhrase>. Spec changes: WebAudio/web-speech-api#169 Bug: 381349238 Change-Id: I9cb4bdafd7c74cf48a4d7f3684af644df1eff4c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6696798 Reviewed-by: Domenic Denicola <[email protected]> Commit-Queue: Evan Liu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1483461} -- wpt-commits: 82b74727278342807e9b42c9f29ecc7bdddc0495 wpt-pr: 53634
This PR replaces SpeechRecognitionPhraseList with ObservableArray.
Closes #168
Preview | Diff