[webkit-reviews] review denied: [Bug 122681] [GTK] Search functionality in MiniBrowser provides feedback on search fail : [Attachment 214034] 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 214034: Patch
https://bugs.webkit.org/attachment.cgi?id=214034&action=review

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


I think it's a good idea, I would just do a small change in the code. See it
below...

> Tools/MiniBrowser/gtk/BrowserSearchBar.c:46
> +static void setEntryBackgroundColor(BrowserSearchBar *searchBar, gboolean
failedSearch)

This function is named setEntryBackgroundColor(), but it does not set any color
at all, just specifies whether the search failed (so the color is set
accordingly).

So, I think you either rename it to something that makes more sense according
to the bool parameter (e.g. setFailedStyleForEntry()), so the bool TRUE means
"failed" and FALSE means "ok/default", or you go further to even avoid the bool
parameter by declaring a new enum with ENTRY_STYLE_DEFAULT/OK/FAIL values (for
instance) to be passed to the function, which maybe I would rename then to
setStyleForEntry().

For MiniBrowser, I do not have any special preference over those. Any option
would be ok to me :)