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.
Created attachment 223572 [details] Patch
Comment on attachment 223572 [details] 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.
Comment on attachment 223572 [details] 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.
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 :)