BookMan: Fixed various issues related to bookmark reloading from file
ClosedPublic

Authored by nmel on Apr 20 2018, 7:16 AM.

Details

Summary

Changed importFromFile function which is called after Bookmark Manager
sends the signal that bookmarks are modified and should be reloaded.
Now we don't delete bookmarks unless they are fake bookmarks (folders or
separators). By keeping the original actions and updating them on reload
we keep bindings with different systems like toolbars and shortcuts.

FIXED: [ 390991 ] bookmark buttons on a toolbar disappear after closing Bookmark Editor
BUG: 390991

FIXED: [ 267719 ] bookmarks shortcuts lost after opening bookmark manager
BUG: 267719

Test Plan

Test that the bugs mentioned in Summary are no longer reproducible and also if everything is fine after editing bookmarks (add bookmark, manage bookmarks).

Please be aware of the following bugs while testing:
https://bugs.kde.org/show_bug.cgi?id=393320 : Editing a local path in Bookmark Manager breaks a bookmark
https://bugs.kde.org/show_bug.cgi?id=393321 : An icon changed in Bookmark Manager is not displayed in the bookmark menu
It should be easy to confirm them - please update the status of the bugs accordingly.

Diff Detail

Repository
R167 Krusader
Branch
fix-bookmark-reloading
Lint
No Linters Available
Unit
No Unit Test Coverage
nmel requested review of this revision.Apr 20 2018, 7:16 AM
nmel created this revision.
nmel updated this revision to Diff 32850.Apr 22 2018, 10:22 PM

Rebased on master.

nmel edited the test plan for this revision. (Show Details)Apr 22 2018, 10:31 PM
nmel added a project: Krusader.
martinkostolny added a subscriber: martinkostolny.

Nicely fixed, thanks Nikita!

Before applying the patch I was able to replicate 390991 but not 267719. Maybe I don't really know what "all shortcus assigned to bookmarks are turned off" actually means. Anyway everything is working fine after the patch :).

This revision is now accepted and ready to land.Apr 24 2018, 4:39 PM
nmel added a comment.Apr 25 2018, 3:31 AM

Thanks for the review and testing, Martin!

Maybe I don't really know what "all shortcus assigned to bookmarks are turned off" actually means. Anyway everything is working fine after the patch :).

It means that you can assign shortcuts to specific bookmarks and those were stopped working after managing. For example, I assigned Ctrl+~ to the root data dir on my machine because it's like a home for the most data. This Ctrl+~ is lost together with toolbar buttons. Now it works without a problem.

This revision was automatically updated to reflect the committed changes.
In D12369#253383, @nmel wrote:

It means that you can assign shortcuts to specific bookmarks and those were stopped working after managing. For example, I assigned Ctrl+~ to the root data dir on my machine because it's like a home for the most data. This Ctrl+~ is lost together with toolbar buttons. Now it works without a problem.

Now I understand! I missed the word 'shortcuts' somehow. Yes, I confirm that this problem was there and is now fixed. Thanks!