GNOME Bugzilla – Bug 720978
add search to cookies dialog
Last modified: 2015-10-07 13:31:50 UTC
It would be nice to have a search similar to the one in passwords in the cookies dialog.
Created attachment 275625 [details] [review] Proposed patch Implemented search in the cookies dialog, the search box filtering cookies by host.
Review of attachment 275625 [details] [review]: This works well. Would be nice to get it in. One of the other developers will want to review it first, though. ::: src/cookies-dialog.c @@ +66,3 @@ gtk_list_store_clear (GTK_LIST_STORE (dialog->priv->liststore)); dialog->priv->filled = FALSE; + Looks like an extra whitespace change @@ +87,3 @@ + g_free (priv->search_text); + priv->search_text = NULL; I think this should be freed in finalize, not dispose (and then you don't have to set it to NULL). @@ +407,3 @@ + -1); + + if (host != NULL && g_strrstr (host, dialog->priv->search_text) != NULL) Not that it makes much difference, but I think it makes sense to use plain strstr() here. Might even be slightly faster since the user will usually be searching for a term from the start of the URL, not the end.
Robert, will you update this at some point? Would be nice to get this in sooner or later.
Comment on attachment 275625 [details] [review] Proposed patch Marking reviewed to get this off the unreviewed patch radar. Please update when you get a chance. :)
Created attachment 305141 [details] [review] Updated proposed patch I somehow missed all pings about this. I'm not entirely sure how, but checking the buglist I've found this again, and I'm updating the patch now. Updated to apply on master and fixed based on review.
Review of attachment 305141 [details] [review]: Sorry for the delay again. This looks good; thanks. I only found one real problem: ::: src/cookies-dialog.c @@ +96,3 @@ + priv = EPHY_COOKIES_DIALOG (object)->priv; + + g_free (priv->search_text); You forgot to chain up to the parent class's finalize! @@ +176,3 @@ + gtk_tree_model_filter_convert_iter_to_child_iter (GTK_TREE_MODEL_FILTER (dialog->priv->treemodelfilter), + &child_iter, + &filter_iter); Nit: the whitespace is off here, by two spaces.
Created attachment 312339 [details] [review] Added search to cookies dialog
Comment on attachment 305141 [details] [review] Updated proposed patch Updated patch attached, marking the old one as obsolete.
Review of attachment 312339 [details] [review]: Thanks, I will commit when we branch. Sorry again this took so long!
(In reply to Michael Catanzaro from comment #9) > Review of attachment 312339 [details] [review] [review]: > > Thanks, I will commit when we branch. Sorry again this took so long! No problem, we both took our times for the answers :) Good that it will be fixed now, after branching.
Attachment 312339 [details] pushed as b93f4d7 - Added search to cookies dialog