[webkit-reviews] review granted: [Bug 254443] AX: Only FRED VP visible on the Text To Speech Sound Check page : [Attachment 465577] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 24 15:58:28 PDT 2023


Tyler Wilcock <tyler_w at apple.com> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 254443: AX: Only FRED VP visible on the Text To Speech Sound Check page
https://bugs.webkit.org/show_bug.cgi?id=254443

Attachment 465577: Patch

https://bugs.webkit.org/attachment.cgi?id=465577&action=review




--- Comment #5 from Tyler Wilcock <tyler_w at apple.com> ---
Comment on attachment 465577
  --> https://bugs.webkit.org/attachment.cgi?id=465577
Patch

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

> COMMIT_MESSAGE:2
> +https://bugs.webkit.org/show_bug.cgi?id=254443

I think you need a Radar link below your Bugzilla URL. If you re-run
`git-webkit setup` this should start appearing in your commit message template.

> LayoutTests/fast/speechsynthesis/mac/samantha-voice-available.html:9
> +    description("This tests that we can get Samantha.");

Can probably move everything inside this script tag back one indentation level.

> LayoutTests/fast/speechsynthesis/mac/samantha-voice-available.html:12
> +    var speech = window.speechSynthesis;
> +    var list = speech.getVoices();

Seems like we only use this `var speech` in one place. Might make the test a
bit more clean to directly use window.speechSynthesis to `getVoices()`.

> LayoutTests/fast/speechsynthesis/mac/samantha-voice-available.html:22
> +

This newline seems unnecessary.


More information about the webkit-reviews mailing list