Clean url's path before initializing KUrlNavigator
Review Request #128334 - Created July 1, 2016 and submitted
|Chinmoy Ranjan Pradhan|
When KUrlNavigator is initialized with a path containing redundant ('/') say '/home/user///folder' then KUrlNavigator::updateButtons fails to create a button for "folder". This happes because of the following lines
const QString dirName = path.section(QLatin1Char('/'), idx, idx);
hasNext = isFirstButton || !dirName.isEmpty();
Here after creating button for "user" idx points to an empty field as a result an empty string is returned to dirName and hasNext evaluates to false. Due to this button for "folder" isn't created.
What i propose is to use QDir::cleanPath in KUrlNavigator::initialize to remove redundant "/" just like it is done in KUrlNavigator::setLocationUrl.
To test this i used the test provided for kurlnavigator in kio. I changed the source of kurlnavigatortes_gui.cpp and initialized kurlnavigator with the path ('/home/chinmoy///Desktop').
Nitpick: then you could even merge the two lines here, it will be more readable ("new" url? what's new about it? in many cases it's the same as url).
data.url = url.adjusted(QUrl::NormalizePathSegments);
Ideally there would be a unittest for this, can you add a testcase with redundant slashes in KUrlNavigatorTest::testButtonUrl_data, maybe? (making sure it fails without your fix, and works with it, of course)
KUrlNavigatorTest::testButtonUrl_data will eventually call KUrlNavigator::setLocationUrl which is not what we want to test. The issue is with KUrlNavigator::initialize which is called only once that too by the constructor.Test for KUrlNavigator::initialize must be done just after the object has been created.So i added a separate test KUrlNavigatorTest::testInitWithRedundantPathSeparator(the name's bit long :P)
First i thought of using KUrlNavigatorTest::m_navigator for the above test. But using that requires certain changes to the KUrlNavigator's constructor as well as four other tests(HistorySizeAndIndex, GoBack, GoForward HistoryInsert).
And with these changes, without the fix, along with my proposed test the four tests mentioned above will also fail which makes no sense as they depend on KUrlNavigator::setLocationUrl(which cleans the url beforehand) and have no connection with KUrlNavigator::initialize.
So KUrlNavigatorTest::testInitWithRedundantPathSeparator creates its own local object (as its the last test) and perform the required test on it and finally deletes it there.