112261 2013-03-13 08:04:04 -0700 [WK2][EFL] Mark not implemented methods in TextCheckerEfl 2013-04-09 00:29:16 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 g.czajkowski g.czajkowski cdumez gyuyoung.kim kenneth lucas.de.marchi mikhail.pozdnyakov rakuco webkit.review.bot oldest_to_newest 854299 0 g.czajkowski 2013-03-13 08:04:04 -0700 As a continuation of bug 111713 I'd like to prose further improvements of EFL's TextChecker. It's no need to call client's methods in TextCheckerEfl if they are not implemented. At the moment, they are using internal WK2 WebTextChecker class wich is not recommended. It's better to mark them as not implemented unless we provide proper implementation. 854300 1 192924 g.czajkowski 2013-03-13 08:07:22 -0700 Created attachment 192924 proposed patch 854304 2 192924 mikhail.pozdnyakov 2013-03-13 08:13:46 -0700 Comment on attachment 192924 proposed patch Makes sense. 854307 3 mikhail.pozdnyakov 2013-03-13 08:15:48 -0700 (In reply to comment #2) > (From update of attachment 192924 [details]) > Makes sense. The class TextChecker is WK2 generic so we cannot just remove those methods, right? 854309 4 g.czajkowski 2013-03-13 08:20:36 -0700 (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 192924 [details] [details]) > > Makes sense. > The class TextChecker is WK2 generic so we cannot just remove those methods, right? Exactly, thanks for the review! 854921 5 192924 gyuyoung.kim 2013-03-13 20:55:55 -0700 Comment on attachment 192924 proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=192924&action=review Should we remove this at the moment ? Is it better to remove when we prepare a patch for new implementation ? > Source/WebKit2/ChangeLog:10 > + class wich is not recommended. Typo: wich ? 855027 6 g.czajkowski 2013-03-14 00:53:05 -0700 (In reply to comment #5) > (From update of attachment 192924 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192924&action=review > > Should we remove this at the moment ? Is it better to remove when we prepare a patch for new implementation ? IMO, it's better to remove it. Grammar checking implementation is more complicated than the spell checking due to libraries/services unavailability. We are not sure whether the implementation will be provided. Secondly our text checker doesn't implement them (the client's callbacks are not initialized) so it doesn't make sense to invoke them. Finally, Kenneth mentioned that we shouldn't call the client's methods in TextCheckerEfl as the matter of fact we use an internal WebKit implementation (with Enchant support). It'd be better to call them directly as Mac is doing in their TextCheckerMac.mm > > > Source/WebKit2/ChangeLog:10 > > + class wich is not recommended. > > Typo: wich ? Thanks. If you don't mind I fix it before landing. 855031 7 gyuyoung.kim 2013-03-14 01:06:35 -0700 Ok, looks make sense. 871521 8 g.czajkowski 2013-04-09 00:29:16 -0700 Committed r147993: <http://trac.webkit.org/changeset/147993> 192924 2013-03-13 08:07:22 -0700 2013-04-08 07:05:51 -0700 proposed patch spelling-mark-notimplemented-methods.patch text/plain 3966 g.czajkowski ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA2M2U2OTljLi4yNTAzNjkzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjMg QEAKKzIwMTMtMDMtMTMgIEdyemVnb3J6IEN6YWprb3dza2kgIDxnLmN6YWprb3dza2lAc2Ftc3Vu Zy5jb20+CisKKyAgICAgICAgW1dLMl1bRUZMXSBNYXJrIG5vdCBpbXBsZW1lbnRlZCBtZXRob2Rz IGluIFRleHRDaGVja2VyRWZsLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTEyMjYxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgSXQncyBubyBuZWVkIHRvIGNhbGwgY2xpZW50J3MgbWV0aG9kcyBpbiBUZXh0 Q2hlY2tlckVmbCBpZiB0aGV5IGFyZSBub3QKKyAgICAgICAgaW1wbGVtZW50ZWQuIEF0IHRoZSBt b21lbnQsIHRoZXkgYXJlIHVzaW5nIGludGVybmFsIFdLMiBXZWJUZXh0Q2hlY2tlcgorICAgICAg ICBjbGFzcyB3aWNoIGlzIG5vdCByZWNvbW1lbmRlZC4KKyAgICAgICAgSXQncyBiZXR0ZXIgdG8g bWFyayB0aGVtIGFzIG5vdCBpbXBsZW1lbnRlZCB1bnRpbCB3ZSBwcm92aWRlIHByb3BlcgorICAg ICAgICBpbXBsZW1lbnRhdGlvbi4KKworICAgICAgICAqIFVJUHJvY2Vzcy9lZmwvVGV4dENoZWNr ZXJFZmwuY3BwOgorICAgICAgICAoV2ViS2l0OjpUZXh0Q2hlY2tlcjo6Y2hlY2tHcmFtbWFyT2ZT dHJpbmcpOgorICAgICAgICAoV2ViS2l0OjpUZXh0Q2hlY2tlcjo6c3BlbGxpbmdVSUlzU2hvd2lu Zyk6CisgICAgICAgIChXZWJLaXQ6OlRleHRDaGVja2VyOjp0b2dnbGVTcGVsbGluZ1VJSXNTaG93 aW5nKToKKyAgICAgICAgKFdlYktpdDo6VGV4dENoZWNrZXI6OnVwZGF0ZVNwZWxsaW5nVUlXaXRo TWlzc3BlbGxlZFdvcmQpOgorICAgICAgICAoV2ViS2l0OjpUZXh0Q2hlY2tlcjo6dXBkYXRlU3Bl bGxpbmdVSVdpdGhHcmFtbWFyU3RyaW5nKToKKwogMjAxMy0wMy0xMyAgSm9jZWx5biBUdXJjb3R0 ZSAgPGpvY2VseW4udHVyY290dGVAZGlnaWEuY29tPgogCiAgICAgICAgIFtXSzJdW1F0XSBSZWdy ZXNzaW9uKDE0NTUxNykgV2ViUHJvY2VzcyBhc3NlcnRzIGluIGRlYnVnIGJ1aWxkIG9uIFdlYktp dFRlc3RSdW5uZXIgc3RhcnQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9l ZmwvVGV4dENoZWNrZXJFZmwuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2VmbC9UZXh0 Q2hlY2tlckVmbC5jcHAKaW5kZXggMjY0NzNkNi4uMTFkYTA5NCAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdDIvVUlQcm9jZXNzL2VmbC9UZXh0Q2hlY2tlckVmbC5jcHAKKysrIGIvU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL2VmbC9UZXh0Q2hlY2tlckVmbC5jcHAKQEAgLTI4LDYgKzI4LDcgQEAK ICNpbmNsdWRlICJjb25maWcuaCIKICNpbmNsdWRlICJUZXh0Q2hlY2tlci5oIgogCisjaW5jbHVk ZSAiTm90SW1wbGVtZW50ZWQuaCIKICNpbmNsdWRlICJUZXh0Q2hlY2tlclN0YXRlLmgiCiAKICNp ZiBFTkFCTEUoU1BFTExDSEVDSykKQEAgLTIxMCw1NSArMjExLDMwIEBAIHZvaWQgVGV4dENoZWNr ZXI6OmNoZWNrU3BlbGxpbmdPZlN0cmluZyhpbnQ2NF90IHNwZWxsRG9jdW1lbnRUYWcsIGNvbnN0 IFVDaGFyKiB0CiAjZW5kaWYKIH0KIAotdm9pZCBUZXh0Q2hlY2tlcjo6Y2hlY2tHcmFtbWFyT2ZT dHJpbmcoaW50NjRfdCBzcGVsbERvY3VtZW50VGFnLCBjb25zdCBVQ2hhciogdGV4dCwgdWludDMy X3QgbGVuZ3RoLCBWZWN0b3I8R3JhbW1hckRldGFpbD4mIGdyYW1tYXJEZXRhaWxzLCBpbnQzMl90 JiBiYWRHcmFtbWFyTG9jYXRpb24sIGludDMyX3QmIGJhZEdyYW1tYXJMZW5ndGgpCit2b2lkIFRl eHRDaGVja2VyOjpjaGVja0dyYW1tYXJPZlN0cmluZyhpbnQ2NF90LCBjb25zdCBVQ2hhciosIHVp bnQzMl90LCBWZWN0b3I8R3JhbW1hckRldGFpbD4mLCBpbnQzMl90JiwgaW50MzJfdCYpCiB7Ci0j aWYgRU5BQkxFKFNQRUxMQ0hFQ0spCi0gICAgV2ViVGV4dENoZWNrZXI6OnNoYXJlZCgpLT5jbGll bnQoKS5jaGVja0dyYW1tYXJPZlN0cmluZyhzcGVsbERvY3VtZW50VGFnLCBTdHJpbmcodGV4dCwg bGVuZ3RoKSwgZ3JhbW1hckRldGFpbHMsIGJhZEdyYW1tYXJMb2NhdGlvbiwgYmFkR3JhbW1hckxl bmd0aCk7Ci0jZWxzZQotICAgIFVOVVNFRF9QQVJBTShzcGVsbERvY3VtZW50VGFnKTsKLSAgICBV TlVTRURfUEFSQU0odGV4dCk7Ci0gICAgVU5VU0VEX1BBUkFNKGxlbmd0aCk7Ci0gICAgVU5VU0VE X1BBUkFNKGdyYW1tYXJEZXRhaWxzKTsKLSAgICBVTlVTRURfUEFSQU0oYmFkR3JhbW1hckxvY2F0 aW9uKTsKLSAgICBVTlVTRURfUEFSQU0oYmFkR3JhbW1hckxlbmd0aCk7Ci0jZW5kaWYKKyAgICBu b3RJbXBsZW1lbnRlZCgpOwogfQogCiBib29sIFRleHRDaGVja2VyOjpzcGVsbGluZ1VJSXNTaG93 aW5nKCkKIHsKLSNpZiBFTkFCTEUoU1BFTExDSEVDSykKLSAgICByZXR1cm4gV2ViVGV4dENoZWNr ZXI6OnNoYXJlZCgpLT5jbGllbnQoKS5zcGVsbGluZ1VJSXNTaG93aW5nKCk7Ci0jZWxzZQorICAg IG5vdEltcGxlbWVudGVkKCk7CiAgICAgcmV0dXJuIGZhbHNlOwotI2VuZGlmCiB9CiAKIHZvaWQg VGV4dENoZWNrZXI6OnRvZ2dsZVNwZWxsaW5nVUlJc1Nob3dpbmcoKQogewotI2lmIEVOQUJMRShT UEVMTENIRUNLKQotICAgIFdlYlRleHRDaGVja2VyOjpzaGFyZWQoKS0+Y2xpZW50KCkudG9nZ2xl U3BlbGxpbmdVSUlzU2hvd2luZygpOwotI2VuZGlmCisgICAgbm90SW1wbGVtZW50ZWQoKTsKIH0K IAotdm9pZCBUZXh0Q2hlY2tlcjo6dXBkYXRlU3BlbGxpbmdVSVdpdGhNaXNzcGVsbGVkV29yZChp bnQ2NF90IHNwZWxsRG9jdW1lbnRUYWcsIGNvbnN0IFN0cmluZyYgbWlzc3BlbGxlZFdvcmQpCit2 b2lkIFRleHRDaGVja2VyOjp1cGRhdGVTcGVsbGluZ1VJV2l0aE1pc3NwZWxsZWRXb3JkKGludDY0 X3QsIGNvbnN0IFN0cmluZyYpCiB7Ci0jaWYgRU5BQkxFKFNQRUxMQ0hFQ0spCi0gICAgV2ViVGV4 dENoZWNrZXI6OnNoYXJlZCgpLT5jbGllbnQoKS51cGRhdGVTcGVsbGluZ1VJV2l0aE1pc3NwZWxs ZWRXb3JkKHNwZWxsRG9jdW1lbnRUYWcsIG1pc3NwZWxsZWRXb3JkKTsKLSNlbHNlCi0gICAgVU5V U0VEX1BBUkFNKHNwZWxsRG9jdW1lbnRUYWcpOwotICAgIFVOVVNFRF9QQVJBTShtaXNzcGVsbGVk V29yZCk7Ci0jZW5kaWYKKyAgICBub3RJbXBsZW1lbnRlZCgpOwogfQogCi12b2lkIFRleHRDaGVj a2VyOjp1cGRhdGVTcGVsbGluZ1VJV2l0aEdyYW1tYXJTdHJpbmcoaW50NjRfdCBzcGVsbERvY3Vt ZW50VGFnLCBjb25zdCBTdHJpbmcmIGJhZEdyYW1tYXJQaHJhc2UsIGNvbnN0IEdyYW1tYXJEZXRh aWwmIGdyYW1tYXJEZXRhaWwpCit2b2lkIFRleHRDaGVja2VyOjp1cGRhdGVTcGVsbGluZ1VJV2l0 aEdyYW1tYXJTdHJpbmcoaW50NjRfdCwgY29uc3QgU3RyaW5nJiwgY29uc3QgR3JhbW1hckRldGFp bCYpCiB7Ci0jaWYgRU5BQkxFKFNQRUxMQ0hFQ0spCi0gICAgV2ViVGV4dENoZWNrZXI6OnNoYXJl ZCgpLT5jbGllbnQoKS51cGRhdGVTcGVsbGluZ1VJV2l0aEdyYW1tYXJTdHJpbmcoc3BlbGxEb2N1 bWVudFRhZywgYmFkR3JhbW1hclBocmFzZSwgZ3JhbW1hckRldGFpbCk7Ci0jZWxzZQotICAgIFVO VVNFRF9QQVJBTShzcGVsbERvY3VtZW50VGFnKTsKLSAgICBVTlVTRURfUEFSQU0oYmFkR3JhbW1h clBocmFzZSk7Ci0gICAgVU5VU0VEX1BBUkFNKGdyYW1tYXJEZXRhaWwpOwotI2VuZGlmCisgICAg bm90SW1wbGVtZW50ZWQoKTsKIH0KIAogdm9pZCBUZXh0Q2hlY2tlcjo6Z2V0R3Vlc3Nlc0Zvcldv cmQoaW50NjRfdCBzcGVsbERvY3VtZW50VGFnLCBjb25zdCBTdHJpbmcmIHdvcmQsIGNvbnN0IFN0 cmluZyYgLCBWZWN0b3I8U3RyaW5nPiYgZ3Vlc3NlcykK