After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 720978 - add search to cookies dialog
add search to cookies dialog
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Passwords, Cookies, & Certificates
git master
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-23 12:50 UTC by William Jon McCann
Modified: 2015-10-07 13:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (16.32 KB, patch)
2014-05-02 09:37 UTC, Robert Roth
none Details | Review
Updated proposed patch (16.54 KB, patch)
2015-06-12 12:35 UTC, Robert Roth
needs-work Details | Review
Added search to cookies dialog (16.61 KB, patch)
2015-09-29 02:27 UTC, Robert Roth
committed Details | Review

Description William Jon McCann 2013-12-23 12:50:30 UTC
It would be nice to have a search similar to the one in passwords in the cookies dialog.
Comment 1 Robert Roth 2014-05-02 09:37:22 UTC
Created attachment 275625 [details] [review]
Proposed patch

Implemented search in the cookies dialog, the search box filtering cookies by host.
Comment 2 Michael Catanzaro 2014-08-17 16:33:34 UTC
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.
Comment 3 Michael Catanzaro 2015-02-26 21:00:02 UTC
Robert, will you update this at some point? Would be nice to get this in sooner or later.
Comment 4 Michael Catanzaro 2015-04-25 15:25:13 UTC
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. :)
Comment 5 Robert Roth 2015-06-12 12:35:55 UTC
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.
Comment 6 Michael Catanzaro 2015-09-10 00:39:40 UTC
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.
Comment 7 Robert Roth 2015-09-29 02:27:22 UTC
Created attachment 312339 [details] [review]
Added search to cookies dialog
Comment 8 Robert Roth 2015-09-29 02:28:51 UTC
Comment on attachment 305141 [details] [review]
Updated proposed patch

Updated patch attached, marking the old one as obsolete.
Comment 9 Michael Catanzaro 2015-09-29 03:03:46 UTC
Review of attachment 312339 [details] [review]:

Thanks, I will commit when we branch. Sorry again this took so long!
Comment 10 Robert Roth 2015-09-29 04:03:14 UTC
(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.
Comment 11 Michael Catanzaro 2015-10-07 13:31:46 UTC
Attachment 312339 [details] pushed as b93f4d7 - Added search to cookies dialog