Don't restore user places with invalid attribute href.
This avoids asking for empty protocol in KFilePlacesItem::groupType and the assert in KProtocolInfoFactory::findProtocol.
Details
- Reviewers
dfaure - Group Reviewers
Frameworks - Commits
- R241:2b52b47211a6: Don't try to restore invalid user places
Don't assert asking for invalid protocols when creating the save dialog with an invalid bookmark (empty href) in ~/.local/share/user-places.xbel
<bookmark href=""> <title>Project Folder</title> <info> <metadata owner="http://freedesktop.org"> <bookmark:icon name="folder-favorites"/> </metadata> <metadata owner="http://www.kde.org"> <OnlyInApp>kdenlive</OnlyInApp> </metadata> </info> </bookmark>
Diff Detail
- Repository
- R241 KIO
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Continue processing the XML with invalid urls.
And a trivial code deduplication, I couldn't resist.
This commit breaks kfileplacesmodeltest.
$ bin/kfileplacesmodeltest testInitialState testInitialList testInternalBookmarksHaveIds
- Start testing of KFilePlacesModelTest *****
Config: Using QtTest library 5.12.0, Qt 5.12.0 (x86_64-little_endian-lp64 shared (dynamic) debug build; by GCC 7.3.1 20180323 [gcc-7-branch revision 258812])
QDEBUG : KFilePlacesModelTest::initTestCase() 18:21:44.046 kfileplacesmodeltest(22234) ?[32mSolid::Backends::Fake::FakeManager::parseMachineFile?[0m void Solid::Backends::Fake::FakeManager::parseMachineFile() Parsing fake computer XML: "/d/kde/src/5/frameworks/kio/autotests/fakecomputer.xml"
PASS : KFilePlacesModelTest::initTestCase()
PASS : KFilePlacesModelTest::testInitialState()
PASS : KFilePlacesModelTest::testInitialList()
FAIL! : KFilePlacesModelTest::testInternalBookmarksHaveIds() '!ids.contains(id)' returned FALSE. (Duplicated ID found!)
Loc: [/d/kde/src/5/frameworks/kio/autotests/kfileplacesmodeltest.cpp(315)]
PASS : KFilePlacesModelTest::cleanupTestCase()
Totals: 4 passed, 1 failed, 0 skipped, 0 blacklisted, 75ms
- Finished testing of KFilePlacesModelTest *****
I have debugged it a little bit but I don't really understand the overall logic.
Many bookmarks have a UDI (like /org/kde/solid/fakehw/fstab/thehost/solidpath) and no URL, and those are the ones that are duplicated now.
When this code is called again, those bookmarks are now skipped in the first iteration, and therefore added by the next foreach (the one which says "Add bookmarks for the remaining devices, they were previous unknown").
Maybe this skipping should only be done for bookmarks with an empty UDI?
More generally, please please run the unittests after changing a class. I hate that I currently am the one doing this, on the day of the release, which forces me to bugfix such things, or delay the release.