134512 2014-07-01 14:32:22 -0700 AX: [iOS WebKit2] Support for Speak selection 2014-07-01 15:26:00 -0700 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 cfleizach cfleizach bunhere cdumez commit-queue gyuyoung.kim mark.lam sergio webkit-bug-importer oldest_to_newest 1019766 0 cfleizach 2014-07-01 14:32:22 -0700 WebKit2 needs support for speak selection <rdar://problem/15212005> 1019770 1 234199 cfleizach 2014-07-01 14:46:44 -0700 Created attachment 234199 patch 1019775 2 234199 darin 2014-07-01 14:51:14 -0700 Comment on attachment 234199 patch View in context: https://bugs.webkit.org/attachment.cgi?id=234199&action=review > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1371 > + _page->getSelectionOrContentsAsString([self](const String& string, CallbackBase::Error error) { > + if (error != CallbackBase::Error::None) > + return; > + if ([self respondsToSelector:@selector(accessibilitySpeakSelectionSetContent:)]) > + [self accessibilitySpeakSelectionSetContent:string]; > + }); Is there a guarantee we won’t get the callback after self is deallocated? Do we need to retain/release self? > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1372 > + Stray blank line. 1019779 3 cfleizach 2014-07-01 15:02:50 -0700 (In reply to comment #2) > (From update of attachment 234199 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234199&action=review > > > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1371 > > + _page->getSelectionOrContentsAsString([self](const String& string, CallbackBase::Error error) { > > + if (error != CallbackBase::Error::None) > > + return; > > + if ([self respondsToSelector:@selector(accessibilitySpeakSelectionSetContent:)]) > > + [self accessibilitySpeakSelectionSetContent:string]; > > + }); > > Is there a guarantee we won’t get the callback after self is deallocated? Do we need to retain/release self? This is basically what define: does as well, so if that case is safe so will this one. Maybe someone else is able to speak to whether the connection callback blocks are torn down safely when WKContentView goes away... > > > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1372 > > + > > Stray blank line. Thanks! 1019787 4 cfleizach 2014-07-01 15:26:00 -0700 http://trac.webkit.org/changeset/170670 234199 2014-07-01 14:46:44 -0700 2014-07-01 14:51:14 -0700 patch patch text/plain 1845 cfleizach SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3MDY2OCkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDE0LTA3LTAxICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgQVg6IFtpT1MgV2ViS2l0 Ml0gU3VwcG9ydCBmb3IgU3BlYWsgc2VsZWN0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzQ1MTIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBJbXBsZW1lbnQgYSBtZXRob2QgdGhhdCBTcGVhayBTZWxl Y3Rpb24gY2FuIHVzZSB0byByZXRyaWV2ZSB0aGUgc2VsZWN0ZWQgY29udGVudC4KKworICAgICAg ICAqIFVJUHJvY2Vzcy9pb3MvV0tDb250ZW50Vmlld0ludGVyYWN0aW9uLm1tOgorICAgICAgICAo LVtXS0NvbnRlbnRWaWV3IGFjY2Vzc2liaWxpdHlSZXRyaWV2ZVNwZWFrU2VsZWN0aW9uQ29udGVu dF0pOgorCiAyMDE0LTA3LTAxICBCcmFkeSBFaWRzb24gIDxiZWlkc29uQGFwcGxlLmNvbT4KIAog ICAgICAgICBGaXggU2VydmljZXNPdmVybGF5Q29udHJvbGxlciB0ZWFyZG93bgpJbmRleDogU291 cmNlL1dlYktpdDIvVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRWaWV3SW50ZXJhY3Rpb24ubW0KPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2lvcy9XS0NvbnRlbnRWaWV3SW50 ZXJhY3Rpb24ubW0JKHJldmlzaW9uIDE3MDY0MykKKysrIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9pb3MvV0tDb250ZW50Vmlld0ludGVyYWN0aW9uLm1tCSh3b3JraW5nIGNvcHkpCkBAIC0xOTEs NiArMTkxLDEwIEBACiAKIEBlbmQKIAorQGludGVyZmFjZSBXS0NvbnRlbnRWaWV3IChXS0ludGVy YWN0aW9uUHJpdmF0ZSkKKy0gKHZvaWQpYWNjZXNzaWJpbGl0eVNwZWFrU2VsZWN0aW9uU2V0Q29u dGVudDooTlNTdHJpbmcgKilzdHJpbmc7CitAZW5kCisKIEBpbXBsZW1lbnRhdGlvbiBXS0NvbnRl bnRWaWV3IChXS0ludGVyYWN0aW9uKQogCiBzdGF0aWMgVUlXZWJTZWxlY3Rpb25Nb2RlIHRvVUlX ZWJTZWxlY3Rpb25Nb2RlKFdLU2VsZWN0aW9uR3JhbnVsYXJpdHkgZ3JhbnVsYXJpdHkpCkBAIC0x MzU3LDYgKzEzNjEsMTcgQEAKICAgICB9KTsKIH0KIAorLSAodm9pZClhY2Nlc3NpYmlsaXR5UmV0 cmlldmVTcGVha1NlbGVjdGlvbkNvbnRlbnQKK3sKKyAgICBfcGFnZS0+Z2V0U2VsZWN0aW9uT3JD b250ZW50c0FzU3RyaW5nKFtzZWxmXShjb25zdCBTdHJpbmcmIHN0cmluZywgQ2FsbGJhY2tCYXNl OjpFcnJvciBlcnJvcikgeworICAgICAgICBpZiAoZXJyb3IgIT0gQ2FsbGJhY2tCYXNlOjpFcnJv cjo6Tm9uZSkKKyAgICAgICAgICAgIHJldHVybjsKKyAgICAgICAgaWYgKFtzZWxmIHJlc3BvbmRz VG9TZWxlY3RvcjpAc2VsZWN0b3IoYWNjZXNzaWJpbGl0eVNwZWFrU2VsZWN0aW9uU2V0Q29udGVu dDopXSkKKyAgICAgICAgICAgIFtzZWxmIGFjY2Vzc2liaWxpdHlTcGVha1NlbGVjdGlvblNldENv bnRlbnQ6c3RyaW5nXTsKKyAgICB9KTsKKworfQorCiAvLyBVSVdLSW50ZXJhY3Rpb25WaWV3UHJv dG9jb2wKIAogc3RhdGljIGlubGluZSBHZXN0dXJlVHlwZSB0b0dlc3R1cmVUeXBlKFVJV0tHZXN0 dXJlVHlwZSBnZXN0dXJlVHlwZSkK