Bug 237614 - AX: Support updated WebSpeech API
Summary: AX: Support updated WebSpeech API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
: 216683 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-03-08 12:07 PST by chris fleizach
Modified: 2022-03-18 11:57 PDT (History)
9 users (show)

See Also:


Attachments
patch (80.15 KB, patch)
2022-03-08 16:32 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (80.58 KB, patch)
2022-03-08 16:59 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (81.55 KB, patch)
2022-03-09 00:12 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (81.70 KB, patch)
2022-03-09 00:26 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (81.49 KB, patch)
2022-03-09 00:27 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (81.49 KB, patch)
2022-03-09 00:29 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (81.36 KB, patch)
2022-03-09 00:30 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (80.83 KB, patch)
2022-03-09 00:31 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (81.33 KB, patch)
2022-03-09 00:49 PST, chris fleizach
andresg_22: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (81.93 KB, patch)
2022-03-09 08:34 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (81.97 KB, patch)
2022-03-09 09:17 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (86.65 KB, patch)
2022-03-09 09:33 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (86.65 KB, patch)
2022-03-09 14:02 PST, chris fleizach
no flags Details | Formatted Diff | Diff
patch (86.62 KB, patch)
2022-03-09 14:11 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (86.62 KB, patch)
2022-03-09 14:19 PST, chris fleizach
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (86.39 KB, patch)
2022-03-09 15:33 PST, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2022-03-08 12:07:15 PST
There are new WebSpeech changes from
https://wicg.github.io/speech-api/

and the lack of these are causing a number of imported W3C test failures

web-platform-tests/speech-api/SpeechSynthesis*
Comment 1 Radar WebKit Bug Importer 2022-03-08 12:07:39 PST
<rdar://problem/89981851>
Comment 2 chris fleizach 2022-03-08 12:08:13 PST
<rdar://problem/88982090>
Comment 3 chris fleizach 2022-03-08 16:32:53 PST
Created attachment 454172 [details]
patch
Comment 4 chris fleizach 2022-03-08 16:59:16 PST
Created attachment 454176 [details]
patch
Comment 5 chris fleizach 2022-03-09 00:12:25 PST
Created attachment 454199 [details]
patch
Comment 6 chris fleizach 2022-03-09 00:26:11 PST
Created attachment 454200 [details]
patch
Comment 7 chris fleizach 2022-03-09 00:27:03 PST
Created attachment 454201 [details]
patch
Comment 8 chris fleizach 2022-03-09 00:29:16 PST
Created attachment 454202 [details]
patch
Comment 9 chris fleizach 2022-03-09 00:30:40 PST
Created attachment 454203 [details]
patch
Comment 10 chris fleizach 2022-03-09 00:31:59 PST
Created attachment 454204 [details]
patch
Comment 11 chris fleizach 2022-03-09 00:49:42 PST
Created attachment 454205 [details]
patch
Comment 12 Andres Gonzalez 2022-03-09 06:42:56 PST
(In reply to chris fleizach from comment #11)
> Created attachment 454205 [details]
> patch

--- a/LayoutTests/fast/speechsynthesis/speech-synthesis-boundary-events.html
+++ a/LayoutTests/fast/speechsynthesis/speech-synthesis-boundary-events.html

-       debug("Boundary event: " + event.name + ", Character index: " + event.charIndex);
+       debug("Boundary event: " + event.name + ", Character index: " + event.charIndex + ", length: " + event.charLength);

This can be written, arguably more compactly as:

+       debug(`Boundary event: ${event.name}, Character index: ${event.charIndex}, length: ${event.charLength}`);

--- a/Source/WebCore/Modules/speech/SpeechSynthesis.cpp
+++ a/Source/WebCore/Modules/speech/SpeechSynthesis.cpp

-void SpeechSynthesis::fireEvent(const AtomString& type, SpeechSynthesisUtterance& utterance, unsigned long charIndex, const String& name)
+void SpeechSynthesis::fireEvent(const AtomString& type, SpeechSynthesisUtterance& utterance, unsigned long charIndex, unsigned long charLength, const String& name) const
 {
-    utterance.dispatchEvent(SpeechSynthesisEvent::create(type, charIndex, (MonotonicTime::now() - utterance.startTime()).seconds(), name));
+    SpeechSynthesisEventInit eventInit = { &utterance, charIndex, charLength, static_cast<float>((MonotonicTime::now() - utterance.startTime()).seconds()), name };
+    utterance.dispatchEvent(SpeechSynthesisEvent::create(type, eventInit));
+}

I'd suggest avoiding the local eventInit and just pass the initializer list as second parameter to SpeechSynthesisEvent::create.

+void SpeechSynthesis::fireErrorEvent(const AtomString& type, SpeechSynthesisUtterance& utterance, SpeechSynthesisErrorCode errorCode) const
+{
+    SpeechSynthesisErrorEventInit eventInit = { { &utterance, 0, 0, static_cast<float>((MonotonicTime::now() - utterance.startTime()).seconds()), { } }, errorCode };
+    utterance.dispatchEvent(SpeechSynthesisErrorEvent::create(type, eventInit));
 }

Same here.

--- a/Source/WebCore/Modules/speech/SpeechSynthesis.h
+++ a/Source/WebCore/Modules/speech/SpeechSynthesis.h

+    virtual ~SpeechSynthesis();

Do we need this declaration and the "~SpeechSynthesis() = default;" in the cpp?

+    using RefCounted::ref;
+    using RefCounted::deref;

Do we need this? I would think this is implicit by deriving from RefCounted.

--- a/Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp
+++ a/Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp

-    client()->boundaryEventOccurred(*m_utterance, SpeechBoundary::SpeechWordBoundary, 0);
-    client()->boundaryEventOccurred(*m_utterance, SpeechBoundary::SpeechSentenceBoundary, m_utterance->text().length());
+    client()->boundaryEventOccurred(*m_utterance, SpeechBoundary::SpeechWordBoundary, 0, 3);

Why 3? Perhaps a clarifying comment.
Comment 13 chris fleizach 2022-03-09 08:34:17 PST
> +    using RefCounted::ref;
> +    using RefCounted::deref;
> 
> Do we need this? I would think this is implicit by deriving from RefCounted.
> 

This inherits from multiple classes that define ref, so we need to specify which one to use. I see this pattern elsewhere too

> --- a/Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp
> +++ a/Source/WebCore/platform/mock/PlatformSpeechSynthesizerMock.cpp
> 
> -    client()->boundaryEventOccurred(*m_utterance,
> SpeechBoundary::SpeechWordBoundary, 0);
> -    client()->boundaryEventOccurred(*m_utterance,
> SpeechBoundary::SpeechSentenceBoundary, m_utterance->text().length());
> +    client()->boundaryEventOccurred(*m_utterance,
> SpeechBoundary::SpeechWordBoundary, 0, 3);
> 
> Why 3? Perhaps a clarifying comment.


will add comment. its just test data so it doesn't have a real impact
Comment 14 chris fleizach 2022-03-09 08:34:28 PST
Created attachment 454242 [details]
patch
Comment 15 chris fleizach 2022-03-09 09:17:21 PST
Created attachment 454247 [details]
patch
Comment 16 chris fleizach 2022-03-09 09:33:04 PST
Created attachment 454250 [details]
patch
Comment 17 chris fleizach 2022-03-09 14:02:17 PST
Created attachment 454281 [details]
patch
Comment 18 chris fleizach 2022-03-09 14:11:54 PST
Created attachment 454283 [details]
patch
Comment 19 chris fleizach 2022-03-09 14:19:21 PST
Created attachment 454287 [details]
patch
Comment 20 chris fleizach 2022-03-09 15:33:38 PST
Created attachment 454293 [details]
patch
Comment 21 EWS 2022-03-10 12:16:51 PST
Committed r291124 (248284@main): <https://commits.webkit.org/248284@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454293 [details].
Comment 22 chris fleizach 2022-03-18 11:57:48 PDT
*** Bug 216683 has been marked as a duplicate of this bug. ***