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

Add initial support for JS SpeechSynthesisErrorEvent #823

Conversation

asurdej-comcast
Copy link

@asurdej-comcast asurdej-comcast commented Apr 6, 2022

  1. Add new type SpeechSynthesisErrorEvent and expose it to JS
  2. Extend PlatformSpeechSynthesizer speakingErrorOccured with error type arg
  3. Send proper JS event on error condiditon
  4. Stick to using older SpeechSynthesisEvent for platforms
    that doesn't support ErrorEvent yeti (mac)

@asurdej-comcast asurdej-comcast force-pushed the speech_synth_error_event branch from 09a1549 to d5e29c3 Compare April 6, 2022 09:42
1) Add new type SpeechSynthesisErrorEvent and expose it to JS
2) Extend PlatformSpeechSynthesizer speakingErrorOccured with error type arg
3) Send proper JS event on error condiditon
4) Stick to using older SpeechSynthesisEvent for platforms
   that doesn't support ErrorEvent yeti (mac)
@asurdej-comcast asurdej-comcast force-pushed the speech_synth_error_event branch from d5e29c3 to 0abbe61 Compare April 6, 2022 09:50
@asurdej-comcast asurdej-comcast marked this pull request as ready for review April 6, 2022 10:05
@woutermeek woutermeek requested a review from lauromoura April 7, 2022 12:41
@lauromoura
Copy link
Member

There's an upstream commit from March in WebKit/WebKit@c8a7a8619b280 that adds a SpeechSynthesisErrorEvent with a slightly different API. For example, in SpeechSynthesisErrorEvent.idl:

<     Conditional=SPEECH_SYNTHESIS
---
>     Conditional=SPEECH_SYNTHESIS,
>     Exposed=Window
45c31,33
<     readonly attribute SpeechSynthesisErrorCode  error;
---
>     constructor(DOMString type, SpeechSynthesisErrorEventInit eventInitDict);
> 
>     readonly attribute SpeechSynthesisErrorCode error;

Among other changes, the upstream version matches the spec at https://wicg.github.io/speech-api/#speechreco-error with an extra implementation-specific messageattribute. So I think it would be nice if we backported it.

I tried applying it but couldn't test properly, as I didn't manage to get speech synthesis enabled in my WPE build (desktop). Is there any pending patch I could use to test it? Or another environment (e.g. buildroot)?

@asurdej-comcast
Copy link
Author

Hi, thanks for your replay. I was not aware of an upstream patch. Indeed my patch misses SpeechSynthesisErrorEventInit dict initializer for ErrorEvent that is present in upstream version. On the other hand the other implementation doesn't allow to trigger different error codes from platform implementation so effectively only peechSynthesisErrorCode::Canceled is used (unless I miss something in the code).
I believe it's reasonable to wait for upstream patch backport and then rebase rest of my changes. Do you have any idea when this could happen roughly?

For the testing, WPE doesn't have any default PlatformSpeechSynthesizer implementation so the code won't be able to compile probably after enabling speech synthesis option. We do have our own implementation based on platform specific stuff but this is very Comcast specific. Currently there is only some mac implementations but not sure if we are able to use this (error event code is generic so any platform would be fine to test) or the mock version just to see the flow...

@woutermeek woutermeek added the wpe-2.28 Only for PR affecting 2.28 label May 4, 2022
@lauromoura
Copy link
Member

Hi, thanks for your replay. I was not aware of an upstream patch. Indeed my patch misses SpeechSynthesisErrorEventInit dict initializer for ErrorEvent that is present in upstream version. On the other hand the other implementation doesn't allow to trigger different error codes from platform implementation so effectively only peechSynthesisErrorCode::Canceled is used (unless I miss something in the code). I believe it's reasonable to wait for upstream patch backport and then rebase rest of my changes. Do you have any idea when this could happen roughly?

Hi @asurdej-comcast sorry for the delay. It's on my list for this week.

For the testing, WPE doesn't have any default PlatformSpeechSynthesizer implementation so the code won't be able to compile probably after enabling speech synthesis option. We do have our own implementation based on platform specific stuff but this is very Comcast specific. Currently there is only some mac implementations but not sure if we are able to use this (error event code is generic so any platform would be fine to test) or the mock version just to see the flow...

I'm thinking of giving a try and adding some mocks just to see if things get compiled to make it easier to track failures.

@lauromoura
Copy link
Member

I've put the WIP backport in https://github.com/lauromoura/WPEWebKit/tree/speech-error-event-backport

And the upstream speech synth stubs in https://github.com/lauromoura/webkit/tree/speech-synthesis-glib-stubs

This Monday I'll test both together.

@lauromoura
Copy link
Member

Updated API PR in #854

@asurdej-comcast
Copy link
Author

#854 has landed
Rebased version of this PR in #863
Closing

Scony added a commit to Scony/WPEWebKit that referenced this pull request Mar 21, 2023
- Add missing functions
- Add proper error handling (similar to WebPlatformForEmbedded#823 with optional as in WebPlatformForEmbedded#875) for both UIProcess and WebProcess
Scony added a commit to Scony/WPEWebKit that referenced this pull request Mar 21, 2023
- Add missing functions
- Add proper error handling (similar to WebPlatformForEmbedded#823 with optional as in WebPlatformForEmbedded#875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Sep 20, 2023
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Sep 29, 2023
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Apr 23, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Apr 23, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Apr 23, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Apr 23, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Apr 24, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Apr 25, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request Apr 30, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
pgorszkowski-igalia pushed a commit that referenced this pull request May 2, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Jun 19, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Sep 5, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Sep 11, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Sep 20, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Sep 26, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Oct 4, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
magomez pushed a commit that referenced this pull request Oct 8, 2024
- Add missing functions
- Add proper error handling (similar to #823 with optional as in #875) for both UIProcess and WebProcess
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wpe-2.28 Only for PR affecting 2.28
Development

Successfully merging this pull request may close these issues.

3 participants