[Webkit-unassigned] [Bug 117631] [GTK] Provide search functionality to MiniBrowser

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 15 00:41:08 PDT 2013


Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
 Attachment #205085|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |

--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-07-15 00:42:14 PST ---
(From update of attachment 205085)
View in context: https://bugs.webkit.org/attachment.cgi?id=205085&action=review

Looks good to me in general, there are just a few minor details. Thanks!

> Tools/MiniBrowser/gtk/BrowserSearchBar.c:60
> +static void searchEntryIconReleasedCallback(BrowserSearchBar *searchBar)

I would call this searchEntryClearIconReleasedCallback to make more obvious this is the callback of the clear icon.

> Tools/MiniBrowser/gtk/BrowserSearchBar.c:146
> +

There's an extra line here.

> Tools/MiniBrowser/gtk/BrowserSearchBar.h:52
> +void browser_search_bar_search_next(BrowserSearchBar *);
> +void browser_search_bar_search_previous(BrowserSearchBar *);

These are not used outside BrowserSearchBar, make them private.

> Tools/MiniBrowser/gtk/BrowserWindow.c:310
> +    browser_search_bar_close(BROWSER_SEARCH_BAR((window->searchBar)));

Extra parentheses there. BROWSER_SEARCH_BAR((

> Tools/MiniBrowser/gtk/BrowserWindow.c:324
> +        browser_search_bar_open(BROWSER_SEARCH_BAR((window->searchBar)));

And here too

> Tools/MiniBrowser/gtk/BrowserWindow.c:583
> +    window->searchBarVisible = FALSE;

You don't need this, this is already FALSE at this point.

> Tools/MiniBrowser/gtk/BrowserWindow.c:627
> +    window->searchBar = GTK_WIDGET(browser_search_bar_new(window->webView));

browser_search_bar_new returns a GtkWidget *, you don't need the cast.

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list