Review Request: Change the way favicons are stored in bookmarks.xml file

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101815/#review6165
-----------------------------------------------------------


Looking at old emails, it seems the icon element is simply missing from the DTD.

Look for icon in http://xbel.sourceforge.net/language/versions/1.0/xbel-1.0.xhtml
See also the discussion at http://sourceforge.net/mailarchive/forum.php?thread_name=20080618085443.GI25433%40klangraum.plasmasturm.org&forum_name=xbel-specs

The right thing to do is to fix the XBEL DTD and add the icon element, given that it's in the xbel-1.0 spec above.

Much better than breaking compatibility with all existing tools which do expect the icon element to be there; this stuff is already hard to make interoperable (one has to find the icon in the icon theme, or worse, find the favicon file in the kde dirs), but changing the way it's stored in the XML will only make this worse.

- David


On July 1, 2011, 2:37 p.m., Filip Brcic wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101815/
> -----------------------------------------------------------
> 
> (Updated July 1, 2011, 2:37 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Currently kde bookmarks keep the favicons in a <bookmark:icon> tag inside the <meta> and that is why the bookmarks.xml file cannot pass the xmllint check.
> 
> This patch proposes a simple change -- favicons are stored as a `favicon' attribute inside the <meta> tag. That change doesn't change any functionality but it enables bookmarks.xml to be in written correct xml.
> 
> Rationale: Why do I care about correct xml? Well, first of all it is nicer to have correct xml, but more importantly, I'm using syncplaces (http://www.andyhalford.com/syncplaces/) addon for firefox to synchronize firefox and konqueror/rekonq bookmarks. If bookmarks are not written in correct xml, the plugin shows some errors and is unable to import & merge the bookmarks. With this little change everything works like a charm.
> 
> 
> Diffs
> -----
> 
>   kio/bookmarks/kbookmark.cc 912052e 
> 
> Diff: http://git.reviewboard.kde.org/r/101815/diff
> 
> 
> Testing
> -------
> 
> I built kdelibs with this patch and I'm running them on my Kubuntu box for a week or so. You can access my patched kdelibs in my ppa on launchpad if you wish: https://launchpad.net/~brcha/+archive/ppa
> 
> Up until now I didn't encounter any problems with this patch. I did reload all favicons, remove bookmarks, and resync them from firefox and everything worked out fine.
> 
> 
> Thanks,
> 
> Filip
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110830/b233b49b/attachment.htm>