| Summary: | AX: Support updated WebSpeech API | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||||||||||||||||||||||||||||||||
| Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | andresg_22, baba, cdumez, esprehn+autocc, ews-watchlist, kangil.han, kondapallykalyan, pascoe, webkit-bug-importer | ||||||||||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||
|
Description
chris fleizach
2022-03-08 12:07:15 PST
Created attachment 454172 [details]
patch
Created attachment 454176 [details]
patch
Created attachment 454199 [details]
patch
Created attachment 454200 [details]
patch
Created attachment 454201 [details]
patch
Created attachment 454202 [details]
patch
Created attachment 454203 [details]
patch
Created attachment 454204 [details]
patch
Created attachment 454205 [details]
patch
(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. > + 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 Created attachment 454242 [details]
patch
Created attachment 454247 [details]
patch
Created attachment 454250 [details]
patch
Created attachment 454281 [details]
patch
Created attachment 454283 [details]
patch
Created attachment 454287 [details]
patch
Created attachment 454293 [details]
patch
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]. *** Bug 216683 has been marked as a duplicate of this bug. *** |