107014 2013-01-16 08:14:34 -0800 WebSpeech: implement voices list 2013-01-17 00:16:08 -0800 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 106742 1 cfleizach cfleizach abarth dmazzoni rniwa oldest_to_newest 808379 0 cfleizach 2013-01-16 08:14:34 -0800 Implement the voices list to return the right data. 808782 1 183044 cfleizach 2013-01-16 15:33:01 -0800 Created attachment 183044 patch 809106 2 183044 abarth 2013-01-16 23:09:35 -0800 Comment on attachment 183044 patch View in context: https://bugs.webkit.org/attachment.cgi?id=183044&action=review I don't read objective-c that well, but the objective-c code looks plausible. If you feel you need an objective-c expert, we might want to ask someone to double-check that part of the patch. > Source/WebCore/Modules/speech/mac/SpeechSynthesisMac.mm:56 > + RefPtr<SpeechSynthesisVoice> voice = SpeechSynthesisVoice::create(voiceURI, name, language, true, isDefault); > + m_voiceList.append(voice); I would combine these lines both for readability and to save a ref churn. You can also save the ref churn by calling voice.release() on the last line. > LayoutTests/platform/mac/TestExpectations:192 > +# Speech synthesis is not yet enabled. > +platform/mac/fast/speechsynthesis Should these tests be in platform/mac, or should they be in fast/speechsynthesis outside the "mac" directory? The test you wrote doesn't seem particularly mac-specific. (I realize this is going to make you edit a bunch of other TestExpectations files, but it's probably the right thing to do.) 809114 3 cfleizach 2013-01-16 23:23:46 -0800 (In reply to comment #2) > (From update of attachment 183044 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=183044&action=review > > I don't read objective-c that well, but the objective-c code looks plausible. If you feel you need an objective-c expert, we might want to ask someone to double-check that part of the patch. > > > Source/WebCore/Modules/speech/mac/SpeechSynthesisMac.mm:56 > > + RefPtr<SpeechSynthesisVoice> voice = SpeechSynthesisVoice::create(voiceURI, name, language, true, isDefault); > > + m_voiceList.append(voice); > > I would combine these lines both for readability and to save a ref churn. You can also save the ref churn by calling voice.release() on the last line. > > > LayoutTests/platform/mac/TestExpectations:192 > > +# Speech synthesis is not yet enabled. > > +platform/mac/fast/speechsynthesis > > Should these tests be in platform/mac, or should they be in fast/speechsynthesis outside the "mac" directory? The test you wrote doesn't seem particularly mac-specific. (I realize this is going to make you edit a bunch of other TestExpectations files, but it's probably the right thing to do.) I thought about that, but given that the data for the voices is going to be platform specific I wasn't sure that i could make a test that would work on all platforms. i figured when the next platform started to implement this we could move the tests that make sense. what do you think? 809130 4 abarth 2013-01-16 23:50:00 -0800 > I thought about that, but given that the data for the voices is going to be platform specific I wasn't sure that i could make a test that would work on all platforms. i figured when the next platform started to implement this we could move the tests that make sense. what do you think? It's hard for me to know without being able to see the tests that we haven't written yet. We can certainly start out this way if you're open to moving the tests in a future patch if they seem useful cross-port. 809135 5 cfleizach 2013-01-17 00:16:08 -0800 https://trac.webkit.org/changeset/139970 183044 2013-01-16 15:33:01 -0800 2013-01-16 23:09:35 -0800 patch patch text/plain 5901 cfleizach SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzOTkzMSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDEzLTAxLTE2ICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgV2ViU3BlZWNoOiBpbXBs ZW1lbnQgdm9pY2VzIGxpc3QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEwNzAxNAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIEFkZCBpbiB0aGUgTWFjIHNpZGUgY29kZSB0byByZXR1cm4gYSBsaXN0IG9mIHZv aWNlcy4gCisgICAgICAgIEFkZHMgYSBsYXlvdXQgdGVzdCwgd2hpY2ggaXMgc2tpcHBlZCBmb3Ig bm93LCB1bnRpbCB0aGUgZmVhdHVyZSBpcyBlbmFibGVkLgorCisgICAgICAgIFRlc3Q6IHBsYXRm b3JtL21hYy9mYXN0L3NwZWVjaHN5bnRoZXNpcy9zcGVlY2gtc3ludGhlc2lzLXZvaWNlcy5odG1s CisKKyAgICAgICAgKiBNb2R1bGVzL3NwZWVjaC9TcGVlY2hTeW50aGVzaXMuY3BwOgorICAgICAg ICAoV2ViQ29yZTo6U3BlZWNoU3ludGhlc2lzOjpTcGVlY2hTeW50aGVzaXMpOgorICAgICAgICAq IE1vZHVsZXMvc3BlZWNoL1NwZWVjaFN5bnRoZXNpcy5oOgorICAgICAgICAoU3BlZWNoU3ludGhl c2lzKToKKyAgICAgICAgKiBNb2R1bGVzL3NwZWVjaC9tYWMvU3BlZWNoU3ludGhlc2lzTWFjLm1t OgorICAgICAgICAoV2ViQ29yZTo6U3BlZWNoU3ludGhlc2lzOjppbml0aWFsaXplVm9pY2VMaXN0 KToKKwogMjAxMy0wMS0xNiAgQmVuamFtaW4gUG91bGFpbiAgPGJwb3VsYWluQGFwcGxlLmNvbT4K IAogICAgICAgICBGb3JjZSBhIHJlYnVpbGQgb2YgUmVuZGVyT2JqZWN0CkluZGV4OiBTb3VyY2Uv V2ViQ29yZS9Nb2R1bGVzL3NwZWVjaC9tYWMvU3BlZWNoU3ludGhlc2lzTWFjLm1tCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9XZWJDb3JlL01vZHVsZXMvc3BlZWNoL21hYy9TcGVlY2hTeW50aGVzaXNN YWMubW0JKHJldmlzaW9uIDEzOTkxOCkKKysrIFNvdXJjZS9XZWJDb3JlL01vZHVsZXMvc3BlZWNo L21hYy9TcGVlY2hTeW50aGVzaXNNYWMubW0JKHdvcmtpbmcgY29weSkKQEAgLTI4LDEwICsyOCwz NSBAQAogCiAjaWYgRU5BQkxFKFNQRUVDSF9TWU5USEVTSVMpCiAKKyNpbmNsdWRlICJTcGVlY2hT eW50aGVzaXNWb2ljZS5oIgogI2luY2x1ZGUgIlNwZWVjaFN5bnRoZXNpc1V0dGVyYW5jZS5oIgor I2luY2x1ZGUgPEFwcEtpdC9OU1NwZWVjaFN5bnRoZXNpemVyLmg+CiAKIG5hbWVzcGFjZSBXZWJD b3JlIHsKICAgICAKK3ZvaWQgU3BlZWNoU3ludGhlc2lzOjppbml0aWFsaXplVm9pY2VMaXN0KCkK K3sKKyAgICBOU1N0cmluZyAqZGVmYXVsdFZvaWNlVVJJID0gW05TU3BlZWNoU3ludGhlc2l6ZXIg ZGVmYXVsdFZvaWNlXTsKKyAgICBOU0FycmF5ICphdmFpbGFibGVWb2ljZXMgPSBbTlNTcGVlY2hT eW50aGVzaXplciBhdmFpbGFibGVWb2ljZXNdOworICAgIE5TVUludGVnZXIgY291bnQgPSBbYXZh aWxhYmxlVm9pY2VzIGNvdW50XTsKKyAgICBmb3IgKE5TVUludGVnZXIgayA9IDA7IGsgPCBjb3Vu dDsgaysrKSB7CisgICAgICAgIE5TU3RyaW5nICp2b2ljZU5hbWUgPSBbYXZhaWxhYmxlVm9pY2Vz IG9iamVjdEF0SW5kZXg6a107CisgICAgICAgIE5TRGljdGlvbmFyeSAqYXR0cmlidXRlcyA9IFtO U1NwZWVjaFN5bnRoZXNpemVyIGF0dHJpYnV0ZXNGb3JWb2ljZTp2b2ljZU5hbWVdOworICAgICAg ICAKKyAgICAgICAgTlNTdHJpbmcgKnZvaWNlVVJJID0gW2F0dHJpYnV0ZXMgb2JqZWN0Rm9yS2V5 Ok5TVm9pY2VJZGVudGlmaWVyXTsKKyAgICAgICAgTlNTdHJpbmcgKm5hbWUgPSBbYXR0cmlidXRl cyBvYmplY3RGb3JLZXk6TlNWb2ljZU5hbWVdOworICAgICAgICBOU1N0cmluZyAqbGFuZ3VhZ2Ug PSBbYXR0cmlidXRlcyBvYmplY3RGb3JLZXk6TlNWb2ljZUxvY2FsZUlkZW50aWZpZXJdOworCisg ICAgICAgIC8vIENoYW5nZSB0byBCQ1AtNDcgZm9ybWF0IGFzIGRlZmluZWQgYnkgc3BlYy4KKyAg ICAgICAgbGFuZ3VhZ2UgPSBbbGFuZ3VhZ2Ugc3RyaW5nQnlSZXBsYWNpbmdPY2N1cnJlbmNlc09m U3RyaW5nOkAiXyIgd2l0aFN0cmluZzpAIi0iXTsKKyAgICAgICAgCisgICAgICAgIGJvb2wgaXNE ZWZhdWx0ID0gW2RlZmF1bHRWb2ljZVVSSSBpc0VxdWFsVG9TdHJpbmc6dm9pY2VVUkldOworICAg ICAgICAKKyAgICAgICAgUmVmUHRyPFNwZWVjaFN5bnRoZXNpc1ZvaWNlPiB2b2ljZSA9IFNwZWVj aFN5bnRoZXNpc1ZvaWNlOjpjcmVhdGUodm9pY2VVUkksIG5hbWUsIGxhbmd1YWdlLCB0cnVlLCBp c0RlZmF1bHQpOworICAgICAgICBtX3ZvaWNlTGlzdC5hcHBlbmQodm9pY2UpOworICAgIH0KK30K KwogYm9vbCBTcGVlY2hTeW50aGVzaXM6OnBlbmRpbmcoKSBjb25zdAogewogICAgIHJldHVybiBm YWxzZTsKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL01vZHVsZXMvc3BlZWNoL1NwZWVjaFN5bnRoZXNp cy5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvTW9kdWxlcy9zcGVlY2gvU3BlZWNo U3ludGhlc2lzLmNwcAkocmV2aXNpb24gMTM5OTE4KQorKysgU291cmNlL1dlYkNvcmUvTW9kdWxl cy9zcGVlY2gvU3BlZWNoU3ludGhlc2lzLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMzcsNiArMzcs NyBAQAogICAgIAogU3BlZWNoU3ludGhlc2lzOjpTcGVlY2hTeW50aGVzaXMoKQogeworICAgIGlu aXRpYWxpemVWb2ljZUxpc3QoKTsKIH0KIAogfSAvLyBuYW1lc3BhY2UgV2ViQ29yZQpJbmRleDog U291cmNlL1dlYkNvcmUvTW9kdWxlcy9zcGVlY2gvU3BlZWNoU3ludGhlc2lzLmgKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL1dlYkNvcmUvTW9kdWxlcy9zcGVlY2gvU3BlZWNoU3ludGhlc2lzLmgJKHJl dmlzaW9uIDEzOTkxOCkKKysrIFNvdXJjZS9XZWJDb3JlL01vZHVsZXMvc3BlZWNoL1NwZWVjaFN5 bnRoZXNpcy5oCSh3b3JraW5nIGNvcHkpCkBAIC01Nyw2ICs1Nyw3IEBACiBwcml2YXRlOgogICAg IFNwZWVjaFN5bnRoZXNpcygpOwogICAgIAorICAgIHZvaWQgaW5pdGlhbGl6ZVZvaWNlTGlzdCgp OwogICAgIFZlY3RvcjxSZWZQdHI8U3BlZWNoU3ludGhlc2lzVm9pY2U+ID4gbV92b2ljZUxpc3Q7 CiB9OwogICAgIApJbmRleDogTGF5b3V0VGVzdHMvcGxhdGZvcm0vbWFjL2Zhc3Qvc3BlZWNoc3lu dGhlc2lzL3NwZWVjaC1zeW50aGVzaXMtdm9pY2VzLWV4cGVjdGVkLnR4dAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9tYWMvZmFzdC9zcGVlY2hzeW50aGVzaXMvc3BlZWNoLXN5 bnRoZXNpcy12b2ljZXMtZXhwZWN0ZWQudHh0CShyZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMv cGxhdGZvcm0vbWFjL2Zhc3Qvc3BlZWNoc3ludGhlc2lzL3NwZWVjaC1zeW50aGVzaXMtdm9pY2Vz LWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSwxMiBAQAorVGhpcyB0ZXN0cyB0 aGF0IHdlIGNhbiBnZXQgc3ludGhlc2l6ZXIgdm9pY2VzIG9uIHRoZSBNYWMKKworT24gc3VjY2Vz cywgeW91IHdpbGwgc2VlIGEgc2VyaWVzIG9mICJQQVNTIiBtZXNzYWdlcywgZm9sbG93ZWQgYnkg IlRFU1QgQ09NUExFVEUiLgorCisKK1BBU1Mgdm9pY2VDb3VudCA+IDAgaXMgdHJ1ZQorUEFTUyBm b3VuZEVuZ2xpc2hWb2ljZSBpcyB0cnVlCitQQVNTIGZvdW5kRGVmYXVsdFZvaWNlIGlzIHRydWUK K1BBU1Mgc3VjY2Vzc2Z1bGx5UGFyc2VkIGlzIHRydWUKKworVEVTVCBDT01QTEVURQorCkluZGV4 OiBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9tYWMvZmFzdC9zcGVlY2hzeW50aGVzaXMvc3BlZWNoLXN5 bnRoZXNpcy12b2ljZXMuaHRtbAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9wbGF0Zm9ybS9t YWMvZmFzdC9zcGVlY2hzeW50aGVzaXMvc3BlZWNoLXN5bnRoZXNpcy12b2ljZXMuaHRtbAkocmV2 aXNpb24gMCkKKysrIExheW91dFRlc3RzL3BsYXRmb3JtL21hYy9mYXN0L3NwZWVjaHN5bnRoZXNp cy9zcGVlY2gtc3ludGhlc2lzLXZvaWNlcy5odG1sCShyZXZpc2lvbiAwKQpAQCAtMCwwICsxLDM5 IEBACis8IURPQ1RZUEUgSFRNTCBQVUJMSUMgIi0vL0lFVEYvL0RURCBIVE1MLy9FTiI+Cis8aHRt bD4KKzxoZWFkPgorPHNjcmlwdCBzcmM9Ii4uLy4uLy4uLy4uL2Zhc3QvanMvcmVzb3VyY2VzL2pz LXRlc3QtcHJlLmpzIj48L3NjcmlwdD4KKzwvaGVhZD4KKzxib2R5IGlkPSJib2R5Ij4KKworPGRp diBpZD0iY29uc29sZSI+PC9kaXY+CisKKzxzY3JpcHQ+CisKKyAgICBkZXNjcmlwdGlvbigiVGhp cyB0ZXN0cyB0aGF0IHdlIGNhbiBnZXQgc3ludGhlc2l6ZXIgdm9pY2VzIG9uIHRoZSBNYWMiKTsK KworICAgIHZhciBzcGVlY2ggPSB3aW5kb3cuc3BlZWNoU3ludGhlc2lzOworICAgIHZhciBsaXN0 ID0gc3BlZWNoLmdldFZvaWNlcygpOworCisgICAgdmFyIGZvdW5kRGVmYXVsdFZvaWNlID0gZmFs c2U7CisgICAgdmFyIGZvdW5kRW5nbGlzaFZvaWNlID0gZmFsc2U7CisgICAgdmFyIHZvaWNlQ291 bnQgPSBsaXN0Lmxlbmd0aDsKKworICAgIGZvciAodmFyIGsgPSAwOyBrIDwgbGlzdC5sZW5ndGg7 IGsrKyApIHsKKyAgICAgICAgdmFyIHZvaWNlID0gbGlzdFtrXTsKKyAgICAgICAgaWYgKHZvaWNl LmxhbmcgPT0gImVuLVVTIikgeworICAgICAgICAgICAgZm91bmRFbmdsaXNoVm9pY2UgPSB0cnVl OworICAgICAgICB9CisgICAgICAgIGlmICh2b2ljZS5kZWZhdWx0KSB7CisgICAgICAgICAgICBm b3VuZERlZmF1bHRWb2ljZSA9IHRydWU7CisgICAgICAgIH0KKyAgICB9CisKKyAgICBzaG91bGRC ZVRydWUoInZvaWNlQ291bnQgPiAwIik7CisgICAgc2hvdWxkQmVUcnVlKCJmb3VuZEVuZ2xpc2hW b2ljZSIpOworICAgIHNob3VsZEJlVHJ1ZSgiZm91bmREZWZhdWx0Vm9pY2UiKTsKKworPC9zY3Jp cHQ+CisKKzxzY3JpcHQgc3JjPSIuLi8uLi8uLi8uLi9mYXN0L2pzL3Jlc291cmNlcy9qcy10ZXN0 LXBvc3QuanMiPjwvc2NyaXB0PgorPC9ib2R5PgorPC9odG1sPgpJbmRleDogTGF5b3V0VGVzdHMv cGxhdGZvcm0vbWFjL1Rlc3RFeHBlY3RhdGlvbnMKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMv cGxhdGZvcm0vbWFjL1Rlc3RFeHBlY3RhdGlvbnMJKHJldmlzaW9uIDEzOTkxNykKKysrIExheW91 dFRlc3RzL3BsYXRmb3JtL21hYy9UZXN0RXhwZWN0YXRpb25zCSh3b3JraW5nIGNvcHkpCkBAIC0x ODgsNiArMTg4LDkgQEAKICMgU3BlZWNoIGlucHV0IGlzIG5vdCB5ZXQgZW5hYmxlZC4KIGZhc3Qv c3BlZWNoCiAKKyMgU3BlZWNoIHN5bnRoZXNpcyBpcyBub3QgeWV0IGVuYWJsZWQuCitwbGF0Zm9y bS9tYWMvZmFzdC9zcGVlY2hzeW50aGVzaXMKKwogIyBTcGVlY2ggSmF2YVNjcmlwdCBBUEkgaXMg bm90IHlldCBlbmFibGVkLgogZmFzdC9ldmVudHMvY29uc3RydWN0b3JzL3NwZWVjaC1yZWNvZ25p dGlvbi1ldmVudC1jb25zdHJ1Y3Rvci5odG1sCiAK