Skip to content
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

Introduce start session algorithm #138

Merged
merged 7 commits into from
Feb 18, 2025
Merged

Conversation

beaufortfrancois
Copy link
Contributor

@beaufortfrancois beaufortfrancois commented Feb 5, 2025

This PR addresses concerns raised in #135 (comment) and #126 (comment):

  • It makes sure start() and start(MediaStreamTrack audioTrack) throw if document is not fully active:
  • Request permission to use microphone is called only for start()
  • A new "start session algorithm" is introduced to formalize how things work when speech recognition starts.

The following tasks have been completed:

Implementation commitment:

  • Blink: (link to issue)
  • Gecko: (link to issue)
  • WebKit: (link to issue)

Preview | Diff

@beaufortfrancois
Copy link
Contributor Author

@evanbliu @padenot @youennf Let me know if that looks good to you

index.bs Outdated
@@ -339,6 +351,16 @@ See <a href="https://lists.w3.org/Archives/Public/public-speech-api/2012Sep/0072

</dl>

<p>When the <dfn>start session algorithm</dfn> with <var>requestMicrophonePermission</var> is invoked, the user agent MUST run the following steps:

1. If the [=current settings object=]'s [=relevant global object=]'s [=associated Document=] is NOT [=fully active=], throw an {{UnknownError}} and abort these steps.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I used UnknownError to match WebKit that already shipped Web Speech.

FYI Firefox uses InvalidStateError instead but did not ship Web Speech. So even though, I'd personally be in favor of InvalidStateError, I picked UnknownError so that web developers already catching this properly based on Safari don't have to update their code.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think webkit could align to whatever deemed appropriate, this is an edge case so this is likely not a web compat issue.
I also prefer InvalidStateError.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you and @evanbliu also prefer InvalidStateError, I've updated this PR to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@youennf This PR has landed and web-platform-tests have been updated. Check out https://wpt.fyi/results/speech-api/SpeechRecognition-detached-iframe.window.html?label=experimental&label=master&aligned
It would be nice to update Safari's implementation as well.

@beaufortfrancois
Copy link
Contributor Author

Quick question for you folks!

When adding web platform tests for this in https://chromium-review.googlesource.com/c/chromium/src/+/6237075, I used frameWindow.SpeechRecognition = frameWindow.SpeechRecognition || frameWindow.webkitSpeechRecognition; so that it works in Safari and Chrome BUT technically, it's not a spec-compliant as webkitSpeechRecognition is not a thing. I still think there's value testing implementation with webkitSpeechRecognition but I'd love your thoughts on this.

@evanbliu
Copy link
Collaborator

Blink owners have requested that we drop the "webkit" prefix from the Web Speech API implementation in Chrome. We'll have to maintain backwards compatibility indefinitely, but eventually we'll probably pretend like it doesn't exist.

@beaufortfrancois
Copy link
Contributor Author

I've addressed @evanbliu's feedback and resolved conflicts. Let me know if there's more to address or if we can merge it.

@beaufortfrancois
Copy link
Contributor Author

@evanbliu Please merge.

@hoch hoch merged commit 6356249 into WebAudio:main Feb 18, 2025
1 check passed
aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 18, 2025
This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <[email protected]>
Reviewed-by: Fr <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1421672}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 18, 2025
This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <[email protected]>
Reviewed-by: Fr <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1421672}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 19, 2025
This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <[email protected]>
Reviewed-by: Fr <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1421672}
aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 19, 2025
Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <[email protected]>
Commit-Queue: Fr <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1422104}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 19, 2025
Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <[email protected]>
Commit-Queue: Fr <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1422104}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 19, 2025
Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <[email protected]>
Commit-Queue: Fr <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1422104}
github-actions bot added a commit to evanbliu/speech-api that referenced this pull request Feb 20, 2025
SHA: 6356249
Reason: push, by evanbliu

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit that referenced this pull request Feb 20, 2025
SHA: 6356249
Reason: push, by hoch

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Feb 26, 2025
…hed context, a=testonly

Automatic update from web-platform-tests
Update Web Speech API to check for detached context

This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <[email protected]>
Reviewed-by: Fr <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1421672}

--

wpt-commits: a2f4f58ea0a30fe00db46db03af3a14ea548d763
wpt-pr: 50786
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Feb 26, 2025
…rame, a=testonly

Automatic update from web-platform-tests
WebSpeech: start() throws if detached iframe

Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <[email protected]>
Commit-Queue: Fr <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1422104}

--

wpt-commits: 247db68758a8b03474678dcc6ecfc1fc955227f6
wpt-pr: 50810
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 27, 2025
…hed context, a=testonly

Automatic update from web-platform-tests
Update Web Speech API to check for detached context

This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <[email protected]>
Reviewed-by: Fr <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1421672}

--

wpt-commits: a2f4f58ea0a30fe00db46db03af3a14ea548d763
wpt-pr: 50786
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 27, 2025
…rame, a=testonly

Automatic update from web-platform-tests
WebSpeech: start() throws if detached iframe

Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <[email protected]>
Commit-Queue: Fr <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1422104}

--

wpt-commits: 247db68758a8b03474678dcc6ecfc1fc955227f6
wpt-pr: 50810
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Mar 1, 2025
…hed context, a=testonly

Automatic update from web-platform-tests
Update Web Speech API to check for detached context

This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <evliugoogle.com>
Reviewed-by: Fr <beaufort.francoisgmail.com>
Cr-Commit-Position: refs/heads/main{#1421672}

--

wpt-commits: a2f4f58ea0a30fe00db46db03af3a14ea548d763
wpt-pr: 50786

UltraBlame original commit: ea0d8b3b9d972a904ea745ca709cf396c67a92ec
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Mar 1, 2025
…rame, a=testonly

Automatic update from web-platform-tests
WebSpeech: start() throws if detached iframe

Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <evliugoogle.com>
Commit-Queue: Fr <beaufort.francoisgmail.com>
Reviewed-by: Mason Freed <masonfchromium.org>
Cr-Commit-Position: refs/heads/main{#1422104}

--

wpt-commits: 247db68758a8b03474678dcc6ecfc1fc955227f6
wpt-pr: 50810

UltraBlame original commit: ea97ba98ce71585e440f2067c143288cea64378b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Mar 1, 2025
…hed context, a=testonly

Automatic update from web-platform-tests
Update Web Speech API to check for detached context

This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <evliugoogle.com>
Reviewed-by: Fr <beaufort.francoisgmail.com>
Cr-Commit-Position: refs/heads/main{#1421672}

--

wpt-commits: a2f4f58ea0a30fe00db46db03af3a14ea548d763
wpt-pr: 50786

UltraBlame original commit: ea0d8b3b9d972a904ea745ca709cf396c67a92ec
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Mar 1, 2025
…rame, a=testonly

Automatic update from web-platform-tests
WebSpeech: start() throws if detached iframe

Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <evliugoogle.com>
Commit-Queue: Fr <beaufort.francoisgmail.com>
Reviewed-by: Mason Freed <masonfchromium.org>
Cr-Commit-Position: refs/heads/main{#1422104}

--

wpt-commits: 247db68758a8b03474678dcc6ecfc1fc955227f6
wpt-pr: 50810

UltraBlame original commit: ea97ba98ce71585e440f2067c143288cea64378b
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Mar 1, 2025
…hed context, a=testonly

Automatic update from web-platform-tests
Update Web Speech API to check for detached context

This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <evliugoogle.com>
Reviewed-by: Fr <beaufort.francoisgmail.com>
Cr-Commit-Position: refs/heads/main{#1421672}

--

wpt-commits: a2f4f58ea0a30fe00db46db03af3a14ea548d763
wpt-pr: 50786

UltraBlame original commit: ea0d8b3b9d972a904ea745ca709cf396c67a92ec
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Mar 1, 2025
…rame, a=testonly

Automatic update from web-platform-tests
WebSpeech: start() throws if detached iframe

Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <evliugoogle.com>
Commit-Queue: Fr <beaufort.francoisgmail.com>
Reviewed-by: Mason Freed <masonfchromium.org>
Cr-Commit-Position: refs/heads/main{#1422104}

--

wpt-commits: 247db68758a8b03474678dcc6ecfc1fc955227f6
wpt-pr: 50810

UltraBlame original commit: ea97ba98ce71585e440f2067c143288cea64378b
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Mar 5, 2025
…hed context, a=testonly

Automatic update from web-platform-tests
Update Web Speech API to check for detached context

This CL updates the new installOnDevice and availableOnDevice APIs of the Web Speech API to check for detached context.

Spec changes: WebAudio/web-speech-api#138

Bug: 40286514
Change-Id: I024a3b906e4d3838fb9c77578a67c57e0249dc2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6259599
Commit-Queue: Evan Liu <[email protected]>
Reviewed-by: Fr <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1421672}

--

wpt-commits: a2f4f58ea0a30fe00db46db03af3a14ea548d763
wpt-pr: 50786
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Mar 5, 2025
…rame, a=testonly

Automatic update from web-platform-tests
WebSpeech: start() throws if detached iframe

Spec PR: WebAudio/web-speech-api#138

Bug: 384797834
Change-Id: I553842e4427071dacc7693ea6f1f2503cb0cf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237075
Reviewed-by: Evan Liu <[email protected]>
Commit-Queue: Fr <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1422104}

--

wpt-commits: 247db68758a8b03474678dcc6ecfc1fc955227f6
wpt-pr: 50810
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants