128459 2014-02-08 10:49:56 -0800 [GTK] Minibrowser: On opening search bar, search for selected text if any 2017-03-11 11:04:58 -0800 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Unspecified Unspecified NEW P2 Minor --- 1 dpino webkit-unassigned agomez berto bugs-noreply cgarcia commit-queue zan oldest_to_newest 978389 0 dpino 2014-02-08 10:49:56 -0800 In all major most browsers, selecting a text and opening the search bar immediately searches for that string in the current page. Implement that functionality in Minibrowser. 978391 1 223572 dpino 2014-02-08 11:07:19 -0800 Created attachment 223572 Patch 993791 2 223572 agomez 2014-03-24 07:45:44 -0700 Comment on attachment 223572 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=223572&action=review > Tools/MiniBrowser/gtk/BrowserSearchBar.c:275 > +static void open_search_bar(BrowserSearchBar *searchBar) Non public API follows WK's C++ style. Hence, the name should be something like openSearchBar. > Tools/MiniBrowser/gtk/BrowserSearchBar.c:286 > +static void get_selected_text_cb(GObject *object, GAsyncResult *result, gpointer userData) Ditto. In addition, the outcome of this cb is to open the search bar regardless to the existence of selected text. Hence, the name of the function is actually misleading. > Tools/MiniBrowser/gtk/BrowserSearchBar.c:293 > + As this should be C++ style, only declare your variables when using, and not sooner. > Tools/MiniBrowser/gtk/BrowserSearchBar.c:298 > + g_error_free(error); Don't we do anything with the error? > Tools/MiniBrowser/gtk/BrowserSearchBar.c:310 > + Same here, don't declare your variables in advance. > Tools/MiniBrowser/gtk/BrowserSearchBar.c:324 > + open_search_bar(searchBar); Just wondering if it wouldn't be better to change the "open_search_bar" API to have a second parameter with the text to set in the entry, instead of doing that here. > Tools/MiniBrowser/gtk/BrowserSearchBar.c:330 > + gchar *script; Do not declare in advance. > Tools/MiniBrowser/gtk/BrowserSearchBar.c:340 > + open_search_bar_and_search_selected_text(searchBar); I would place here the content of this function and remove it completely. It is an unneeded indirection. 1151979 3 223572 mcatanzaro 2016-01-02 09:31:36 -0800 Comment on attachment 223572 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=223572&action=review Thanks for the review, Berto. Updating the flags to make it official and remove this from request queue.... > Tools/MiniBrowser/gtk/BrowserSearchBar.c:322 > + g_free(selectedText); Surely you need to free selectedText even if it's zero-length, since you allocated the memory. 1152267 4 dpino 2016-01-04 00:06:53 -0800 I'm afraid that I won't be able to work on this patch in the short/medium term. If anyone would like to take it over I'm happy with that :) 223572 2014-02-08 11:07:19 -0800 2016-01-02 09:31:36 -0800 Patch bug-128459-20140208200717.patch text/plain 5317 dpino U3VidmVyc2lvbiBSZXZpc2lvbjogMTYzNzEwCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYTdmMzAwZmU1M2QxMTBkNjQyNzU4ZTg4NjkwMThmYTU5 M2E1MGNjMC4uZDYyNjYwYTUyMWY5NTJkMzk5ZWM1NzA3NTJjNDYwZjY1ODRlYzZiMyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDMw IEBACiAyMDE0LTAyLTA4ICBEaWVnbyBQaW5vIEdhcmNpYSAgPGRwaW5vQGlnYWxpYS5jb20+CiAK KyAgICAgICAgW0dUS10gTWluaWJyb3dzZXI6IE9uIG9wZW5pbmcgdGhlIHNlYXJjaCBiYXIsIHNl YXJjaCBmb3Igc2VsZWN0ZWQgdGV4dCBpZiBhbnkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTEyODQ1OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFdoZW4gb3BlbmluZyB0aGUgc2VhcmNoIGJveCwgY2hlY2sg aWYgdGhlcmUncyBhbnkgdGV4dCBzZWxlY3RlZC4gSW4KKyAgICAgICAgdGhhdCBjYXNlIHVwZGF0 ZSB0aGUgc2VhcmNoIGJveCB3aXRoIHRoZSBzZWxlY3RlZCB0ZXh0LiBUaGUgc2VhcmNoIGJveAor ICAgICAgICBhdXRvbWF0aWNhbGx5IHBlcmZvcm1zIGEgc2VhcmNoIHdoZW4gaXQncyBvcGVuZWQu CisKKyAgICAgICAgKiBNaW5pQnJvd3Nlci9ndGsvQnJvd3NlclNlYXJjaEJhci5jOgorICAgICAg ICAob3Blbl9zZWFyY2hfYmFyKToKKyAgICAgICAgT3BlbnMgdGhlIHNlYXJjaCBiYXIgYW5kIHBl cmZvcm1zIGEgc2VhcmNoLgorICAgICAgICAoZ2V0X3NlbGVjdGVkX3RleHRfY2IpOgorICAgICAg ICBSZXRyaWV2ZXMgdGhlIHRleHQgc2VsZWN0ZWQsIHVwZGF0ZXMgdGhlIHNlYXJjaCBib3ggYW5k IGNhbGxzCisgICAgICAgIG9wZW5fc2VhcmNoX2Jhci4KKyAgICAgICAgKG9wZW5fc2VhcmNoX2Jh cl9hbmRfc2VhcmNoX3NlbGVjdGVkX3RleHQpOgorICAgICAgICBFeGVjdXRlcyBKYXZhc2NyaXB0 IGNvZGUgdG8gZ2V0IHRoZSBzZWxlY3RlZCB0ZXh0LCBnZXRfc2VsZWN0ZWRfdGV4dF9jYiAKKyAg ICAgICAgaGFuZGxlcyB0aGUgY2FsbGJhY2suCisgICAgICAgIChicm93c2VyX3NlYXJjaF9iYXJf b3Blbik6CisgICAgICAgIEV4dGVybmFsIGZ1bmN0aW9uLiBDYWxscyBvcGVuX3NlYXJjaF9iYXJf YW5kX3NlYXJjaF9zZWxlY3RlZF90ZXh0LgorICAgICAgICAqIE1pbmlCcm93c2VyL2d0ay9HTlVt YWtlZmlsZS5hbToKKyAgICAgICAgQWRkIGxpYmphdmFzY3JpcHRjb3JlZ3RrIHRvIE1pbmlCcm93 c2VyIExEIGZsYWdzLgorCisyMDE0LTAyLTA4ICBEaWVnbyBQaW5vIEdhcmNpYSAgPGRwaW5vQGln YWxpYS5jb20+CisKICAgICAgICAgW0dUS10gTWluaWJyb3dzZXI6IE1hdGNoZWQgd29yZHMgb2Yg YSBzZWFyY2hlZCB0ZXh0IGFyZSBub3QgaGlnaGxpZ2h0ZWQgYWZ0ZXIgcmVvcGVuaW5nIHNlYXJj aCBiYXIKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEy ODM4MAogCmRpZmYgLS1naXQgYS9Ub29scy9NaW5pQnJvd3Nlci9ndGsvQnJvd3NlclNlYXJjaEJh ci5jIGIvVG9vbHMvTWluaUJyb3dzZXIvZ3RrL0Jyb3dzZXJTZWFyY2hCYXIuYwppbmRleCBhMjJi NTRhYjc4NjA5ZWRhMGE4Y2Q3ODhlYTc3YWViOTk1M2VlYjUxLi4zNzViNmUyMzEzNDUxY2ZhNmMz MzgzOTAyN2QyNzljZGE0NDI2ODE0IDEwMDY0NAotLS0gYS9Ub29scy9NaW5pQnJvd3Nlci9ndGsv QnJvd3NlclNlYXJjaEJhci5jCisrKyBiL1Rvb2xzL01pbmlCcm93c2VyL2d0ay9Ccm93c2VyU2Vh cmNoQmFyLmMKQEAgLTI1LDYgKzI1LDggQEAKIAogI2luY2x1ZGUgIkJyb3dzZXJTZWFyY2hCYXIu aCIKIAorI2luY2x1ZGUgPEphdmFTY3JpcHRDb3JlL0pTU3RyaW5nUmVmLmg+CisjaW5jbHVkZSA8 c3RyaW5nLmg+CiAKIHN0YXRpYyBjb25zdCBjaGFyICpzZWFyY2hFbnRyeUZhaWxlZFN0eWxlID0g Ikd0a0VudHJ5I3NlYXJjaEVudHJ5IHtiYWNrZ3JvdW5kLWNvbG9yOiAjZmY2NjY2O30iOwogCkBA IC0yNzAsMTAgKzI3Miw4IEBAIHZvaWQgYnJvd3Nlcl9zZWFyY2hfYmFyX2FkZF9hY2NlbGVyYXRv cnMoQnJvd3NlclNlYXJjaEJhciAqc2VhcmNoQmFyLCBHdGtBY2NlbEdyCiAgICAgZ3RrX3dpZGdl dF9hZGRfYWNjZWxlcmF0b3Ioc2VhcmNoQmFyLT5wcmV2QnV0dG9uLCAiY2xpY2tlZCIsIGFjY2Vs R3JvdXAsIEdES19LRVlfRywgR0RLX1NISUZUX01BU0sgfCBHREtfQ09OVFJPTF9NQVNLLCBHVEtf QUNDRUxfVklTSUJMRSk7CiB9CiAKLXZvaWQgYnJvd3Nlcl9zZWFyY2hfYmFyX29wZW4oQnJvd3Nl clNlYXJjaEJhciAqc2VhcmNoQmFyKQorc3RhdGljIHZvaWQgb3Blbl9zZWFyY2hfYmFyKEJyb3dz ZXJTZWFyY2hCYXIgKnNlYXJjaEJhcikKIHsKLSAgICBnX3JldHVybl9pZl9mYWlsKEJST1dTRVJf SVNfU0VBUkNIX0JBUihzZWFyY2hCYXIpKTsKLQogICAgIEd0a0VudHJ5ICplbnRyeSA9IEdUS19F TlRSWShzZWFyY2hCYXItPmVudHJ5KTsKIAogICAgIGd0a193aWRnZXRfc2hvdyhHVEtfV0lER0VU KHNlYXJjaEJhcikpOwpAQCAtMjgzLDYgKzI4Myw2MyBAQCB2b2lkIGJyb3dzZXJfc2VhcmNoX2Jh cl9vcGVuKEJyb3dzZXJTZWFyY2hCYXIgKnNlYXJjaEJhcikKICAgICAgICAgZG9TZWFyY2goc2Vh cmNoQmFyKTsKIH0KIAorc3RhdGljIHZvaWQgZ2V0X3NlbGVjdGVkX3RleHRfY2IoR09iamVjdCAq b2JqZWN0LCBHQXN5bmNSZXN1bHQgKnJlc3VsdCwgZ3BvaW50ZXIgdXNlckRhdGEpCit7CisgICAg V2ViS2l0SmF2YXNjcmlwdFJlc3VsdCAqanNSZXN1bHQ7CisgICAgSlNWYWx1ZVJlZiB2YWx1ZTsK KyAgICBKU0dsb2JhbENvbnRleHRSZWYgY29udGV4dDsKKyAgICBHRXJyb3IgKmVycm9yID0gTlVM TDsKKyAgICBnY2hhciAqc2VsZWN0ZWRUZXh0ID0gTlVMTDsKKworICAgIEJyb3dzZXJTZWFyY2hC YXIgKnNlYXJjaEJhciA9IEJST1dTRVJfU0VBUkNIX0JBUih1c2VyRGF0YSk7CisKKyAgICBqc1Jl c3VsdCA9IHdlYmtpdF93ZWJfdmlld19ydW5famF2YXNjcmlwdF9maW5pc2goV0VCS0lUX1dFQl9W SUVXKG9iamVjdCksIHJlc3VsdCwgJmVycm9yKTsKKyAgICBpZiAoIWpzUmVzdWx0KSB7CisgICAg ICAgIGdfZXJyb3JfZnJlZShlcnJvcik7CisgICAgICAgIG9wZW5fc2VhcmNoX2JhcihzZWFyY2hC YXIpOworICAgICAgICByZXR1cm47CisgICAgfQorCisgICAgY29udGV4dCA9IHdlYmtpdF9qYXZh c2NyaXB0X3Jlc3VsdF9nZXRfZ2xvYmFsX2NvbnRleHQoanNSZXN1bHQpOworICAgIHZhbHVlID0g d2Via2l0X2phdmFzY3JpcHRfcmVzdWx0X2dldF92YWx1ZShqc1Jlc3VsdCk7CisKKyAgICAvLyBD b252ZXJ0IEphdmFzY3JpcHQgdmFsdWUgdG8gc3RyaW5nLgorICAgIGlmIChKU1ZhbHVlSXNTdHJp bmcoY29udGV4dCwgdmFsdWUpKSB7CisgICAgICAgIEpTU3RyaW5nUmVmIGpzU3RyVmFsdWU7Cisg ICAgICAgIGdzaXplIGxlbmd0aDsKKworICAgICAgICBqc1N0clZhbHVlID0gSlNWYWx1ZVRvU3Ry aW5nQ29weShjb250ZXh0LCB2YWx1ZSwgTlVMTCk7CisgICAgICAgIGxlbmd0aCA9IEpTU3RyaW5n R2V0TWF4aW11bVVURjhDU3RyaW5nU2l6ZShqc1N0clZhbHVlKTsKKyAgICAgICAgc2VsZWN0ZWRU ZXh0ID0gKGdjaGFyICopIGdfbWFsbG9jKGxlbmd0aCk7CisgICAgICAgIEpTU3RyaW5nR2V0VVRG OENTdHJpbmcoanNTdHJWYWx1ZSwgc2VsZWN0ZWRUZXh0LCBsZW5ndGgpOworICAgICAgICBKU1N0 cmluZ1JlbGVhc2UoanNTdHJWYWx1ZSk7CisgICAgfQorCisgICAgaWYgKHNlbGVjdGVkVGV4dCAm JiBzdHJsZW4oc2VsZWN0ZWRUZXh0KSkgeworICAgICAgICAvLyBVcGRhdGUgc2VhcmNoQmFyIHdp dGggc2VsZWN0ZWQgdGV4dC4KKyAgICAgICAgR3RrRW50cnkgKmVudHJ5ID0gR1RLX0VOVFJZKHNl YXJjaEJhci0+ZW50cnkpOworICAgICAgICBndGtfZW50cnlfc2V0X3RleHQoZW50cnksIHNlbGVj dGVkVGV4dCk7CisgICAgICAgIGdfZnJlZShzZWxlY3RlZFRleHQpOworICAgIH0KKyAgICBvcGVu X3NlYXJjaF9iYXIoc2VhcmNoQmFyKTsKKyAgICB3ZWJraXRfamF2YXNjcmlwdF9yZXN1bHRfdW5y ZWYoanNSZXN1bHQpOworfQorCitzdGF0aWMgdm9pZCBvcGVuX3NlYXJjaF9iYXJfYW5kX3NlYXJj aF9zZWxlY3RlZF90ZXh0KEJyb3dzZXJTZWFyY2hCYXIgKnNlYXJjaEJhcikKK3sKKyAgICBnY2hh ciAqc2NyaXB0OworCisgICAgc2NyaXB0ID0gZ19zdHJkdXBfcHJpbnRmKCJ3aW5kb3cuZ2V0U2Vs ZWN0aW9uKCkudG9TdHJpbmcoKTsiKTsKKyAgICB3ZWJraXRfd2ViX3ZpZXdfcnVuX2phdmFzY3Jp cHQoc2VhcmNoQmFyLT53ZWJWaWV3LCBzY3JpcHQsIE5VTEwsIGdldF9zZWxlY3RlZF90ZXh0X2Ni LCBzZWFyY2hCYXIpOworICAgIGdfZnJlZShzY3JpcHQpOworfQorCit2b2lkIGJyb3dzZXJfc2Vh cmNoX2Jhcl9vcGVuKEJyb3dzZXJTZWFyY2hCYXIgKnNlYXJjaEJhcikKK3sKKyAgICBnX3JldHVy bl9pZl9mYWlsKEJST1dTRVJfSVNfU0VBUkNIX0JBUihzZWFyY2hCYXIpKTsKKyAgICBvcGVuX3Nl YXJjaF9iYXJfYW5kX3NlYXJjaF9zZWxlY3RlZF90ZXh0KHNlYXJjaEJhcik7Cit9CisKIHZvaWQg YnJvd3Nlcl9zZWFyY2hfYmFyX2Nsb3NlKEJyb3dzZXJTZWFyY2hCYXIgKnNlYXJjaEJhcikKIHsK ICAgICBnX3JldHVybl9pZl9mYWlsKEJST1dTRVJfSVNfU0VBUkNIX0JBUihzZWFyY2hCYXIpKTsK ZGlmZiAtLWdpdCBhL1Rvb2xzL01pbmlCcm93c2VyL2d0ay9HTlVtYWtlZmlsZS5hbSBiL1Rvb2xz L01pbmlCcm93c2VyL2d0ay9HTlVtYWtlZmlsZS5hbQppbmRleCA4YmRiZGQzM2U5ZDhmM2NkYjkz MWMzMDFlNmNkY2VmNmNmMTJiNGZmLi4zNmE5Njk0YmIxMzJhYTFiOWY0NjdlYmE0NjlhOTcyYmI0 YTQyZTMyIDEwMDY0NAotLS0gYS9Ub29scy9NaW5pQnJvd3Nlci9ndGsvR05VbWFrZWZpbGUuYW0K KysrIGIvVG9vbHMvTWluaUJyb3dzZXIvZ3RrL0dOVW1ha2VmaWxlLmFtCkBAIC0zOSw2ICszOSw3 IEBAIG5vZGlzdF9Qcm9ncmFtc19NaW5pQnJvd3Nlcl9TT1VSQ0VTID0gXAogCiBQcm9ncmFtc19N aW5pQnJvd3Nlcl9MREFERCA9IFwKIAlsaWJ3ZWJraXQyZ3RrLUBXRUJLSVRHVEtfQVBJX01BSk9S X1ZFUlNJT05ALkBXRUJLSVRHVEtfQVBJX01JTk9SX1ZFUlNJT05ALmxhIFwKKwlsaWJqYXZhc2Ny aXB0Y29yZWd0ay1AV0VCS0lUR1RLX0FQSV9NQUpPUl9WRVJTSU9OQC5AV0VCS0lUR1RLX0FQSV9N SU5PUl9WRVJTSU9OQC5sYSBcCiAJJChGUkVFVFlQRV9MSUJTKSBcCiAJJChHTElCX0xJQlMpIFwK IAkkKEdUS19MSUJTKSBcCg==