Summary: | REGRESSION (iOS 13.4.1): SpeechSynthesisUtterance.onend event won't fire on cancel(). | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Takao Baba <baba> | ||||||||
Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, bfulgham, cfleizach, pvollan, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari 13 | ||||||||||
Hardware: | All | ||||||||||
OS: | iOS 13 | ||||||||||
Attachments: |
|
(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/ Created attachment 399215 [details]
testcase2
OK, I've understood that "onerror" should be fired rather than "onend". But neither onend nor onerror seem be called (see testcase2). (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 Created attachment 399247 [details]
patch
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 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 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 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. (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 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. (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. Committed r261984: <https://trac.webkit.org/changeset/261984> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399247 [details]. (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? (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 Can the synthesizer be invoked multiple times in parallel without flaking? (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 |
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.)