Review Request 122459: sync the bookmarkmanager with the shared one only if change was from this process

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122459/#review75596
-----------------------------------------------------------

Ship it!


I don't know this code very well, but the reasoning seems to make sense - it's slow and pointless to have all processes write the change to the xbel file.

This breaks the theoretical case where kfileplaces/bookmarks.xml is modified by some other app (or code) without modifying user-places.xbel, but I guess that never happens.

It's just funny that the secondary export file (for sharing with other desktops) ends up being used as the trigger for changes in our main file (kfileplaces/bookmarks.xml). But why not.


src/filewidgets/kfileplacessharedbookmarks.cpp
<https://git.reviewboard.kde.org/r/122459/#comment52287>

    probably don't want to leave that enabled


- David Faure


On Feb. 6, 2015, 5:53 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122459/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2015, 5:53 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 343735
>     https://bugs.kde.org/show_bug.cgi?id=343735
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> This is an attempt to fix a bug that seems quite difficult to reproduce, but definitely happens
> It seems there is a weird race condition linked to the sync of the two files
> .local/share/user-places.xbel
> .local/share/kfileplaces/bookmarks.xml
> causing the bug
> https://bugs.kde.org/show_bug.cgi?id=343735
> all processes that are listening for changes in the bookmark manager will try to sync their bookmarks with the shared ones (user-places.xbel) and depending what process arrives first, some bookmarks may be lost
> 
> 
> if this is ok, it will have to go in kdelibs4 (since distributions will support kde4 apps for a long time still)
> as well as (ouch) dolphin both 4 and 5, since it forks KFilePlacesModel
> 
> 
> Diffs
> -----
> 
>   src/filewidgets/kfileplacesmodel.cpp d8da868 
>   src/filewidgets/kfileplacessharedbookmarks.cpp fbd6b53 
>   src/filewidgets/kfileplacessharedbookmarks_p.h 4c9f6a4 
> 
> Diff: https://git.reviewboard.kde.org/r/122459/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150208/ea7a7f2f/attachment.html>