Bug 211776 - REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on cancel().
Summary: REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on c...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Safari 13
Hardware: All iOS 13
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-12 03:44 PDT by Takao Baba
Modified: 2020-05-21 10:50 PDT (History)
5 users (show)

See Also:


Attachments
testcase (1.03 KB, text/html)
2020-05-12 03:44 PDT, Takao Baba
no flags Details
testcase2 (956 bytes, text/html)
2020-05-12 18:09 PDT, Takao Baba
no flags Details
patch (1.90 KB, patch)
2020-05-13 01:01 PDT, 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 Takao Baba 2020-05-12 03:44:54 PDT
Created attachment 399117 [details]
testcase

Step to reproduce:
1. Open attached test.html
2. Click "play" button.
3. Click "stop" button before the speaking finished.

Expected behavior:
An alert "on end" should be shown.

Actual result:
No alert is shown.

UA:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1 Safari/605.1.15

Remarks:
This issue occurs on iOS 13.4.1, while iOS 13.3.1 does not have the issue. Able to reproduce on both simulator and real device.

(Probably Safari on macOS has same issue, but details have not been confirmed.)
Comment 1 Radar WebKit Bug Importer 2020-05-12 03:45:03 PDT
<rdar://problem/63130249>
Comment 2 chris fleizach 2020-05-12 08:03:13 PDT
(In reply to Takao Baba from comment #0)
> Created attachment 399117 [details]
> testcase
> 
> Step to reproduce:
> 1. Open attached test.html
> 2. Click "play" button.
> 3. Click "stop" button before the speaking finished.
> 
> Expected behavior:
> An alert "on end" should be shown.
> 
> Actual result:
> No alert is shown.
> 
> UA:
> Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/605.1.15 (KHTML,
> like Gecko) Version/13.1 Safari/605.1.15
> 
> Remarks:
> This issue occurs on iOS 13.4.1, while iOS 13.3.1 does not have the issue.
> Able to reproduce on both simulator and real device.
> 
> (Probably Safari on macOS has same issue, but details have not been
> confirmed.)

It looks like "onerror" is fired when a cancel event is generated. I think this is correct based on spec usage


4.2.7. SpeechSynthesisErrorEvent Attributes

The SpeechSynthesisErrorEvent is the interface used for the SpeechSynthesisUtterance error event.

error attribute, of type SpeechSynthesisErrorCode, readonly
The errorCode is an enumeration indicating what has gone wrong. The values are:
"canceled"
A cancel method call caused the SpeechSynthesisUtterance to be removed from the queue before it had begun being spoken.

https://wicg.github.io/speech-api/
Comment 3 Takao Baba 2020-05-12 18:09:25 PDT
Created attachment 399215 [details]
testcase2
Comment 4 Takao Baba 2020-05-12 18:12:18 PDT
OK, I've understood that "onerror" should be fired rather than "onend".

But neither onend nor onerror seem be called (see testcase2).
Comment 5 chris fleizach 2020-05-13 00:27:22 PDT
(In reply to Takao Baba from comment #4)
> OK, I've understood that "onerror" should be fired rather than "onend".
> 
> But neither onend nor onerror seem be called (see testcase2).

Yep looks like a regression
Comment 6 chris fleizach 2020-05-13 01:01:21 PDT
Created attachment 399247 [details]
patch
Comment 7 Alexey Proskuryakov 2020-05-15 12:31:35 PDT
Comment on attachment 399247 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399247&action=review

> Source/WebCore/ChangeLog:3
> +        REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on cancel().

Obvious question: can a regression test be made for this issue?
Comment 8 chris fleizach 2020-05-15 12:33:24 PDT
Comment on attachment 399247 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399247&action=review

>> Source/WebCore/ChangeLog:3
>> +        REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on cancel().
> 
> Obvious question: can a regression test be made for this issue?

we have one for the Mock synthesizer that goes all through WebCore. This one uses the synthesizer in the client, which AFAICT there were no tests added for.

@Per, @Brent, is there a way to test this flow? Right now we use a mock synthesizer in these tests that bypasses this logic that was fixed
Comment 9 Brent Fulgham 2020-05-15 16:05:19 PDT
Comment on attachment 399247 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399247&action=review

>>> Source/WebCore/ChangeLog:3
>>> +        REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on cancel().
>> 
>> Obvious question: can a regression test be made for this issue?
> 
> we have one for the Mock synthesizer that goes all through WebCore. This one uses the synthesizer in the client, which AFAICT there were no tests added for.
> 
> @Per, @Brent, is there a way to test this flow? Right now we use a mock synthesizer in these tests that bypasses this logic that was fixed

I do not know of a way to test the system speech synthesizer flow.
Comment 10 Per Arne Vollan 2020-05-15 16:16:22 PDT
Comment on attachment 399247 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399247&action=review

R=me.

>>> Source/WebCore/ChangeLog:3
>>> +        REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on cancel().
>> 
>> Obvious question: can a regression test be made for this issue?
> 
> we have one for the Mock synthesizer that goes all through WebCore. This one uses the synthesizer in the client, which AFAICT there were no tests added for.
> 
> @Per, @Brent, is there a way to test this flow? Right now we use a mock synthesizer in these tests that bypasses this logic that was fixed

I am not sure how to write a test for this. Is it possible to write a layout test that does not use a mock synthesizer?

> Source/WebCore/ChangeLog:17
> +        (WebCore::SpeechSynthesis::cancel):

Perhaps you could include an explanation here if creating a test is not feasible.
Comment 11 chris fleizach 2020-05-15 16:21:53 PDT
(In reply to Per Arne Vollan from comment #10)
> Comment on attachment 399247 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399247&action=review
> 
> R=me.
> 
> >>> Source/WebCore/ChangeLog:3
> >>> +        REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on cancel().
> >> 
> >> Obvious question: can a regression test be made for this issue?
> > 
> > we have one for the Mock synthesizer that goes all through WebCore. This one uses the synthesizer in the client, which AFAICT there were no tests added for.
> > 
> > @Per, @Brent, is there a way to test this flow? Right now we use a mock synthesizer in these tests that bypasses this logic that was fixed
> 
> I am not sure how to write a test for this. Is it possible to write a layout
> test that does not use a mock synthesizer?

There's some reason we didn't do this, but I can't remember now. Will try to research

> 
> > Source/WebCore/ChangeLog:17
> > +        (WebCore::SpeechSynthesis::cancel):
> 
> Perhaps you could include an explanation here if creating a test is not
> feasible.

Yes
Comment 12 Alexey Proskuryakov 2020-05-15 16:58:54 PDT
Perhaps it could be mocked at a lower level? Trying to actually make sound through device's speakers wouldn't be OK :)

Definitely not blocking this fix.
Comment 13 chris fleizach 2020-05-20 22:23:14 PDT
(In reply to Alexey Proskuryakov from comment #12)
> Perhaps it could be mocked at a lower level? Trying to actually make sound
> through device's speakers wouldn't be OK :)
> 

Right, that might be why we moved away from using the actual synthesizer!

> Definitely not blocking this fix.
Comment 14 EWS 2020-05-20 22:46:57 PDT
Committed r261984: <https://trac.webkit.org/changeset/261984>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399247 [details].
Comment 15 Per Arne Vollan 2020-05-21 10:18:02 PDT
(In reply to chris fleizach from comment #13)
> (In reply to Alexey Proskuryakov from comment #12)
> > Perhaps it could be mocked at a lower level? Trying to actually make sound
> > through device's speakers wouldn't be OK :)
> > 
> 
> Right, that might be why we moved away from using the actual synthesizer!
> 
> > Definitely not blocking this fix.

Would it be possible to mute the sound?
Comment 16 chris fleizach 2020-05-21 10:36:33 PDT
(In reply to Per Arne Vollan from comment #15)
> (In reply to chris fleizach from comment #13)
> > (In reply to Alexey Proskuryakov from comment #12)
> > > Perhaps it could be mocked at a lower level? Trying to actually make sound
> > > through device's speakers wouldn't be OK :)
> > > 
> > 
> > Right, that might be why we moved away from using the actual synthesizer!
> > 
> > > Definitely not blocking this fix.
> 
> Would it be possible to mute the sound?

I think we could set the volume to 0 on all the speech requests
Comment 17 Alexey Proskuryakov 2020-05-21 10:49:38 PDT
Can the synthesizer be invoked multiple times in parallel without flaking?
Comment 18 chris fleizach 2020-05-21 10:50:20 PDT
(In reply to Alexey Proskuryakov from comment #17)
> Can the synthesizer be invoked multiple times in parallel without flaking?

on iOS yes, on macOS I am unsure