[webkit-reviews] review denied: [Bug 122681] [GTK] Search functionality in MiniBrowser provides feedback on search fail : [Attachment 217799] Patch

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has denied Andres Gomez Garcia
<agomez at igalia.com>'s request for review:
Bug 122681: [GTK] Search functionality in MiniBrowser provides feedback on
search fail
https://bugs.webkit.org/show_bug.cgi?id=122681

Attachment 217799: Patch
https://bugs.webkit.org/attachment.cgi?id=217799&action=review

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=217799&action=review


Sorry Andres not to have catched the following issues in the previous review.
Looks good anyway, just "one more thing"...

> Tools/MiniBrowser/gtk/BrowserSearchBar.c:35
> +    GtkCssProvider *cssProvider;
> +    const gchar* failStyle;

Wrong * location for failStyle.

Also, I think that we normally use char instead of gchar for internal variables


> Tools/MiniBrowser/gtk/BrowserSearchBar.c:51
> +    if (failedSearch)
> +	   gtk_css_provider_load_from_data(searchBar->cssProvider,
searchBar->failStyle, -1, NULL);
> +    else
> +	   gtk_css_provider_load_from_data(searchBar->cssProvider, "", -1,
NULL);

That's more of a personal preference I think, but I'd use the ternary operator
here

> Tools/MiniBrowser/gtk/BrowserSearchBar.c:169
> +    searchBar->failStyle = " GtkEntry#searchEntry {\n"
> +	   "   background-color: #ff6666;\n"
> +	   "}\n";

What about putting this string in one line only (no \n) and moving its
definition to a global const (static const char*) up in this file, right after
the priv struct?