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 330679 - Epiphany only supports HTTP Local bookmarks (zeroconf)
Epiphany only supports HTTP Local bookmarks (zeroconf)
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Bookmarks
1.9.x
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Marco Pesenti Gritti
: 331022 341353 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2006-02-10 15:01 UTC by Sebastien Bacher
Modified: 2006-05-14 19:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
New version of the patch (2.74 KB, patch)
2006-02-20 21:52 UTC, Celso Pinto
needs-work Details | Review
new patch, it's also more easy to add supported protocols (3.43 KB, patch)
2006-02-23 00:52 UTC, Celso Pinto
none Details | Review
this patch fixes dns_sd discovery not starting (3.52 KB, patch)
2006-02-23 02:08 UTC, Celso Pinto
none Details | Review
and this one fixes the crash on exit (3.54 KB, patch)
2006-02-23 02:18 UTC, Celso Pinto
needs-work Details | Review
Remove Local bookmarks folder (3.56 KB, patch)
2006-02-27 01:34 UTC, Celso Pinto
none Details | Review
new patch for cvs HEAD (3.80 KB, patch)
2006-05-11 12:23 UTC, Christian Persch
committed Details | Review

Description Sebastien Bacher 2006-02-10 15:01:40 UTC
That bug has been described on https://launchpad.net/distros/ubuntu/+source/epiphany-browser/+bug/29893

"Currently, Epiphany only supports HTTP Local bookmarks. Local bookmarks are bookmarks discovered by DNS-SD.

I've created a patch that enables Epiphany to also use HTTPS and FTP.

http://librarian.launchpad.net/1539046/epiphany_dnssd.patch
This patch enables bookmarks that point to discovered HTTPS and FTP network services"
Comment 1 Christian Persch 2006-02-10 16:20:49 UTC
Thanks for the patch! It's a good idea to enable https and ftp shares too.

Now for some problems with the patch:

- it uses gchar; but ephy uses char only

- get_protocol_for_service_type returns newly allocated memory, but could just as well return static strings, or even an enum type

- indentation if off: epiphany uses tabs (8-spaces wide)
+  protocol = get_protocol_for_service_type(service->type);
+  if (protocol == NULL) return;

- you need to use a separate browse handle for each of the supported types, here:
+browse_dns_sd_service(EphyBookmarks *bookmarks,
+      const gchar *protocol)
 {
+  
 	EphyBookmarksPrivate *priv = bookmarks->priv;
-
 	if (gnome_vfs_dns_sd_browse (&priv->browse_handle,
-				     "local", "_http._tcp",
+				     "local", protocol,
 				     (GnomeVFSDNSSDBrowseCallback) browse_cb,

and then cancelling those handles on exit needs to be adapted to cancel all of them.
Comment 2 Christian Persch 2006-02-15 20:54:10 UTC
*** Bug 331022 has been marked as a duplicate of this bug. ***
Comment 3 Celso Pinto 2006-02-20 21:52:28 UTC
Created attachment 59793 [details] [review]
New version of the patch
Comment 4 Celso Pinto 2006-02-20 21:53:56 UTC
Hi chpe, can you please check if the patch I submitted if fine by your coding standards?
Comment 5 Christian Persch 2006-02-21 13:16:10 UTC
Thanks for the new patch!

You seem to have used spaces for indentation somewhere, but we use tab (8 spaces wide).

+        gchar *protocol;
use char*

The ephy_local_bookmarks_init / browse_dns_sd_service change is not correct, you're still overwriting priv->browse_handle twice. You should make priv->browse_handle an array probably.
Comment 6 Celso Pinto 2006-02-23 00:51:31 UTC
chpe, thanks for your mentoring. I've double-checked the new patch and I think it now meets the project's coding standards. 

If there's anything wrong with it, let me know. 
Comment 7 Celso Pinto 2006-02-23 00:52:37 UTC
Created attachment 59965 [details] [review]
new patch, it's also more easy to add supported protocols
Comment 8 Celso Pinto 2006-02-23 02:08:13 UTC
Created attachment 59972 [details] [review]
this patch fixes dns_sd discovery not starting

Forgot to start dns_sd discovery in the previous patch
Comment 9 Celso Pinto 2006-02-23 02:18:30 UTC
Created attachment 59974 [details] [review]
and this one fixes the crash on exit

This patch fixes Epiphany crashing on exit, a bug introduced by previous patches. Sorry...
Comment 10 Christian Persch 2006-02-26 22:49:54 UTC
ephy_node_remove_child (priv->keywords, priv->local) needs to be moved out of browse_dns_sd_service since this should only be done if all 3 of the browse_dns_sd_service calls fail.
Comment 11 Celso Pinto 2006-02-26 23:59:28 UTC
hmmmm doesn't ephy_node_remove_child remove the bookmark-to-be of the list? Or does that remove the "Local" bookmarks folder?
Comment 12 Celso Pinto 2006-02-27 01:34:40 UTC
Created attachment 60188 [details] [review]
Remove Local bookmarks folder

This patch removes the Local bookmarks folder only if browsing of all supported protocols fail.
Comment 13 Crispin Flowerday (not receiving bugmail) 2006-05-11 07:39:17 UTC
*** Bug 341353 has been marked as a duplicate of this bug. ***
Comment 14 Crispin Flowerday (not receiving bugmail) 2006-05-11 07:40:21 UTC
Adjusting the title so that I can actually find the bug if I search for zeroconf, and also mention _https._tcp so that that gets picked up by a search too
Comment 15 Christian Persch 2006-05-11 12:23:50 UTC
Created attachment 65238 [details] [review]
new patch for cvs HEAD
Comment 16 Christian Persch 2006-05-14 19:39:41 UTC
Fixed in the development version. The fix will be available in the next major release. Thank you for your bug report.

2006-05-14  Christian Persch  <chpe@cvs.gnome.org>

        * src/bookmarks/ephy-bookmarks.c: (resolve_cb),
        (ephy_local_bookmarks_init), (ephy_local_bookmarks_stop):

        Add https and ftp local bookmarks. Based on a patch by Celso Pinto,
        bug #330679.