77747 2012-02-03 09:37:50 -0800 [WK2] FindController::hideFindUI should unmark highlighted text matches 2012-06-22 02:54:42 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 svillar svillar andersca cgarcia darin gustavo svillar webkit.review.bot oldest_to_newest 548797 0 svillar 2012-02-03 09:37:50 -0800 Since http://trac.webkit.org/changeset/105855 ports can request WebCore to highlight text matches. Things is that there is no way to unmark all those text matches using WK2 API. That's why I suggest to call unmarkAllTextMatches() in FindController::hideFindUI(). It was not needed before r105855 because the code was unconditionally not highlighting text matches. 548802 1 125354 svillar 2012-02-03 09:42:02 -0800 Created attachment 125354 Patch 549995 2 svillar 2012-02-06 07:45:59 -0800 (In reply to comment #1) > Created an attachment (id=125354) [details] > Patch Do not take into account the comment in ChangeLog about the lack of tests, it's a copy&pasted from other bug. I'm still trying to figure out how to test this (maybe some kind of screenshot...). Suggestions welcomed. 552192 3 svillar 2012-02-08 09:12:25 -0800 (In reply to comment #2) > (In reply to comment #1) > > Created an attachment (id=125354) [details] [details] > > Patch > > Do not take into account the comment in ChangeLog about the lack of tests, it's a copy&pasted from other bug. I'm still trying to figure out how to test this (maybe some kind of screenshot...). Suggestions welcomed. Actually I am not sure that we could do a proper test case for this since the highlight/unhighlight should be covered by a layout test, but we cannot trigger that API from a layout test. Not sure what do you think about landing this one without a test. 552194 4 darin 2012-02-08 09:15:12 -0800 (In reply to comment #3) > Actually I am not sure that we could do a proper test case for this since the highlight/unhighlight should be covered by a layout test, but we cannot trigger that API from a layout test. The normal approach in this situation is to add the ability to trigger the API from a layout test. > Not sure what do you think about landing this one without a test. Usually not a good idea. 554131 5 svillar 2012-02-10 09:57:20 -0800 (In reply to comment #4) > (In reply to comment #3) > > Actually I am not sure that we could do a proper test case for this since the highlight/unhighlight should be covered by a layout test, but we cannot trigger that API from a layout test. > > The normal approach in this situation is to add the ability to trigger the API from a layout test. > > > Not sure what do you think about landing this one without a test. > > Usually not a good idea. So finally I managed to implement a unit test for this new behaviour. That unit test is implemented in the last version of the patch for http://webkit.org/b/76070 but I left it commented. So if the patch for this bug is OK for you then we could land it after 76070 adding the uncommenting of the pending tests (similar to what is planed for http://webkit.org/b/76522). 567724 6 129473 svillar 2012-02-29 10:43:35 -0800 Created attachment 129473 New version of the patch with a proper unit test 567783 7 webkit.review.bot 2012-02-29 11:42:17 -0800 Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API 655325 8 129473 cgarcia 2012-06-22 02:31:29 -0700 Comment on attachment 129473 New version of the patch with a proper unit test Patch looks sane to me. 655334 9 svillar 2012-06-22 02:54:42 -0700 Committed r121015: <http://trac.webkit.org/changeset/121015> 125354 2012-02-03 09:42:02 -0800 2012-02-29 10:43:28 -0800 Patch bug-77747-20120203184200.patch text/plain 1631 svillar U3VidmVyc2lvbiBSZXZpc2lvbjogMTA2NDc4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggYzY5YzIwZGI5MzgxZWYy MzNmMDVhMmQ5ZDIzZjU3NDYyM2JhNzU4MC4uMTJmNDhjMzNmNWQ1YTFjNmNkZGVlYzM0M2Y3YmRm MjQ1OGM2MGUwYyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEyLTAyLTAzICBTZXJn aW8gVmlsbGFyIFNlbmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgorCisgICAgICAgIFtXSzJdIEZp bmRDb250cm9sbGVyOjpoaWRlRmluZFVJIHNob3VsZCB1bm1hcmsgaGlnaGxpZ2h0ZWQgdGV4dCBt YXRjaGVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03 Nzc0NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFVu bWFyayBhbGwgdGV4dCBtYXRjaGVzIHdoZW5ldmVyIEZpbmRDb250cm9sbGVyOjpoaWRlRmluZFVJ IGlzCisgICAgICAgIGNhbGxlZCB0byBhbGxvdyBjYWxsZXJzIHVzaW5nIHRoZSBTaG93SGlnaGxp Z2h0IGZpbmQgb3B0aW9uIHRvCisgICAgICAgIHJlbW92ZSBoaWdobGlnaHRpbmcuCisKKyAgICAg ICAgTm8gbmV3IHRlc3RzIHJlcXVpcmVkLCBhcyB0aGVyZSBpcyBubyBjaGFuZ2UgaW4gZnVuY3Rp b25hbGl0eSwKKyAgICAgICAgd2hpY2ggaXMgYWxyZWFkeSBjb3ZlcmVkIGJ5IGVkaXRpbmcvdGV4 dC1pdGVyYXRvci9maW5kU3RyaW5nLmh0bWwuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlBh Z2UvRmluZENvbnRyb2xsZXIuY3BwOgorICAgICAgICAoV2ViS2l0OjpGaW5kQ29udHJvbGxlcjo6 aGlkZUZpbmRVSSk6CisKIDIwMTItMDEtMzEgIEFsZXhleSBQcm9za3VyeWFrb3YgIDxhcEBhcHBs ZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lPTiAoV2ViS2l0Mik6IGV2ZW50LmtleUNvZGUgaXMg YWx3YXlzIHplcm8gd2hlbiB0eXBpbmcgaW4gUnVzc2lhbgpkaWZmIC0tZ2l0IGEvU291cmNlL1dl YktpdDIvV2ViUHJvY2Vzcy9XZWJQYWdlL0ZpbmRDb250cm9sbGVyLmNwcCBiL1NvdXJjZS9XZWJL aXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9GaW5kQ29udHJvbGxlci5jcHAKaW5kZXggMTBjZmRmYzgz YmU3MTI2MzBhODI1ODZjNWUwYjA2MjA4NDhiOGY5NS4uYzQ3ZjllOThmNzQ2Nzg5NzEzMmFiNmQ0 YzkyYmQ3Y2Q5ZDEyNjk5ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9X ZWJQYWdlL0ZpbmRDb250cm9sbGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNz L1dlYlBhZ2UvRmluZENvbnRyb2xsZXIuY3BwCkBAIC0xNTgsNiArMTU4LDcgQEAgdm9pZCBGaW5k Q29udHJvbGxlcjo6aGlkZUZpbmRVSSgpCiAgICAgaWYgKG1fZmluZFBhZ2VPdmVybGF5KQogICAg ICAgICBtX3dlYlBhZ2UtPnVuaW5zdGFsbFBhZ2VPdmVybGF5KG1fZmluZFBhZ2VPdmVybGF5LCBm YWxzZSk7CiAKKyAgICBtX3dlYlBhZ2UtPmNvcmVQYWdlKCktPnVubWFya0FsbFRleHRNYXRjaGVz KCk7CiAgICAgaGlkZUZpbmRJbmRpY2F0b3IoKTsKIH0KIAo= 129473 2012-02-29 10:43:35 -0800 2012-06-22 02:31:29 -0700 New version of the patch with a proper unit test bug-77747-20120229194333.patch text/plain 3103 svillar U3VidmVyc2lvbiBSZXZpc2lvbjogMTA5MjI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggYjQ4N2VmNTZiZDc5YmY4 MmE4ZTRkNDQ3YTNlNjRmMjIwMTZhNDc2Ny4uZWU1MjY4NGJiNDVkYzI0YTQ4OWRhNmFjYWJkNTUy ODRmMGJiZjg2YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDI0IEBACiAyMDEyLTAyLTI5ICBTZXJn aW8gVmlsbGFyIFNlbmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgogCisgICAgICAgIFtXSzJdIEZp bmRDb250cm9sbGVyOjpoaWRlRmluZFVJIHNob3VsZCB1bm1hcmsgaGlnaGxpZ2h0ZWQgdGV4dCBt YXRjaGVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03 Nzc0NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFVu bWFyayBhbGwgdGV4dCBtYXRjaGVzIHdoZW5ldmVyIEZpbmRDb250cm9sbGVyOjpoaWRlRmluZFVJ IGlzCisgICAgICAgIGNhbGxlZCB0byBhbGxvdyBjYWxsZXJzIHVzaW5nIHRoZSBTaG93SGlnaGxp Z2h0IGZpbmQgb3B0aW9uIHRvCisgICAgICAgIHJlbW92ZSBoaWdobGlnaHRpbmcuCisKKyAgICAg ICAgVGhpcyBwYXRjaCBlbmFibGVzIGEgdW5pdCB0ZXN0IGZvciB0aGUgV2ViS2l0RmluZENvbnRy b2xsZXIKKyAgICAgICAgcHJldmlvdXNseSBndWFyZGVkIGJ5IGEgI2lmKDApIGFmdGVyIHIxMDky MjIuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvQVBJL2d0ay90ZXN0cy9UZXN0V2ViS2l0RmluZENv bnRyb2xsZXIuY3BwOgorICAgICAgICAodGVzdEZpbmRDb250cm9sbGVySGlkZSk6CisgICAgICAg ICogV2ViUHJvY2Vzcy9XZWJQYWdlL0ZpbmRDb250cm9sbGVyLmNwcDoKKyAgICAgICAgKFdlYktp dDo6RmluZENvbnRyb2xsZXI6OmhpZGVGaW5kVUkpOgorCisyMDEyLTAyLTI5ICBTZXJnaW8gVmls bGFyIFNlbmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgorCiAgICAgICAgIERpZEZpbmRTdHJpbmcg c2hvdWxkIGJlIGVtaXR0ZWQgZXZlbiBpZiBGaW5kT3B0aW9uc1Nob3dPdmVybGF5IGlzIG5vdCBl bmFibGVkCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03 NjUyMgogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2d0ay90ZXN0 cy9UZXN0V2ViS2l0RmluZENvbnRyb2xsZXIuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNz L0FQSS9ndGsvdGVzdHMvVGVzdFdlYktpdEZpbmRDb250cm9sbGVyLmNwcAppbmRleCBhZTlmZjJm N2FkOGU1MGQ2NjVhNjc2ZmU4Nzk3MTJmMWViY2E1MTRkLi5hMTg1ZGI4YTc3MWVjZDA3Y2U2Njcy NTMzYjExZDE2ZGY0ZmNkNjU1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv QVBJL2d0ay90ZXN0cy9UZXN0V2ViS2l0RmluZENvbnRyb2xsZXIuY3BwCisrKyBiL1NvdXJjZS9X ZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZ3RrL3Rlc3RzL1Rlc3RXZWJLaXRGaW5kQ29udHJvbGxlci5j cHAKQEAgLTMyNCw4ICszMjQsNiBAQCBzdGF0aWMgdm9pZCB0ZXN0RmluZENvbnRyb2xsZXJIaWRl KEZpbmRDb250cm9sbGVyVGVzdCogdGVzdCwgZ2NvbnN0cG9pbnRlcikKICAgICBnX2Fzc2VydCho aWdobGlnaHRQaXhidWYpOwogICAgIGdfYXNzZXJ0KCFnZGtQaXhidWZFcXVhbChvcmlnaW5hbFBp eGJ1Zi5nZXQoKSwgaGlnaGxpZ2h0UGl4YnVmLmdldCgpKSk7CiAKLSNpZiAoMCkKLSAgICAvLyBS ZXF1aXJlcyBodHRwOi8vd2Via2l0Lm9yZy9iLzc3NzQ3IHRvIGJlIGZpeGVkCiAgICAgV2ViS2l0 RmluZENvbnRyb2xsZXIqIGZpbmRDb250cm9sbGVyID0gd2Via2l0X3dlYl92aWV3X2dldF9maW5k X2NvbnRyb2xsZXIodGVzdC0+bV93ZWJWaWV3KTsKICAgICB3ZWJraXRfZmluZF9jb250cm9sbGVy X3NlYXJjaF9maW5pc2goZmluZENvbnRyb2xsZXIpOwogICAgIHdlYmtpdF93ZWJfdmlld19leGVj dXRlX2VkaXRpbmdfY29tbWFuZCh0ZXN0LT5tX3dlYlZpZXcsICJVbnNlbGVjdCIpOwpAQCAtMzM0 LDcgKzMzMiw2IEBAIHN0YXRpYyB2b2lkIHRlc3RGaW5kQ29udHJvbGxlckhpZGUoRmluZENvbnRy b2xsZXJUZXN0KiB0ZXN0LCBnY29uc3Rwb2ludGVyKQogICAgIEdSZWZQdHI8R2RrUGl4YnVmPiB1 bmhpZ2hsaWdodFBpeGJ1ZiA9IGdka19waXhidWZfZ2V0X2Zyb21fd2luZG93KHdlYlZpZXdHZGtX aW5kb3csIDAsIDAsIGFsbG9jYXRlZEhlaWdodCwgYWxsb2NhdGVkV2lkdGgpOwogICAgIGdfYXNz ZXJ0KHVuaGlnaGxpZ2h0UGl4YnVmKTsKICAgICBnX2Fzc2VydChnZGtQaXhidWZFcXVhbChvcmln aW5hbFBpeGJ1Zi5nZXQoKSwgdW5oaWdobGlnaHRQaXhidWYuZ2V0KCkpKTsKLSNlbmRpZgogfQog CiBzdGF0aWMgdm9pZCB0ZXN0RmluZENvbnRyb2xsZXJJbnN0YW5jZShGaW5kQ29udHJvbGxlclRl c3QqIHRlc3QsIGdjb25zdHBvaW50ZXIpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQ cm9jZXNzL1dlYlBhZ2UvRmluZENvbnRyb2xsZXIuY3BwIGIvU291cmNlL1dlYktpdDIvV2ViUHJv Y2Vzcy9XZWJQYWdlL0ZpbmRDb250cm9sbGVyLmNwcAppbmRleCA2YjYxNTY5MzRjY2IzZjFiMWNj NjFjYzlkOGNlYzY5NWMwNmQ5MDBlLi4wYjc0YzUyYTFiNTdkMWM1NTk3OTVlODJiNzJjNGU0OTY4 NzNkYjAzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2UvRmlu ZENvbnRyb2xsZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9G aW5kQ29udHJvbGxlci5jcHAKQEAgLTE1OSw2ICsxNTksNyBAQCB2b2lkIEZpbmRDb250cm9sbGVy OjpoaWRlRmluZFVJKCkKICAgICBpZiAobV9maW5kUGFnZU92ZXJsYXkpCiAgICAgICAgIG1fd2Vi UGFnZS0+dW5pbnN0YWxsUGFnZU92ZXJsYXkobV9maW5kUGFnZU92ZXJsYXksIGZhbHNlKTsKIAor ICAgIG1fd2ViUGFnZS0+Y29yZVBhZ2UoKS0+dW5tYXJrQWxsVGV4dE1hdGNoZXMoKTsKICAgICBo aWRlRmluZEluZGljYXRvcigpOwogfQogCg==