Summary: | Entering applications:// crashes konqueror | ||
---|---|---|---|
Product: | [Frameworks and Libraries] kdelibs | Reporter: | Albert Astals Cid <aacid> |
Component: | general | Assignee: | kdelibs bugs <kdelibs-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | crash | CC: | ereslibre, faure |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Compiled Sources | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Attachments: |
Patch for fixing the crash
Patch improved for fixing the crash. Patch improved for fixing the crash |
Description
Albert Astals Cid
2007-12-29 17:08:04 UTC
Created attachment 22768 [details]
Patch for fixing the crash
This patch will fix the crash. The problem remains on setEncodedUrl with
QUrl::TolerantMode. I don't know if this should be reported to TT, since maybe
they are doing fine. Anyway comments that deserve being told: setEncodedUrl
will leave "protocol:/" as it is, and "protocol:///" will be converted to
"protocol:/". Maybe there is a rationale behind the behavior of converting
"protocol://" to "protocol:" (what makes it an invalid URL for us, and thus,
crashing). Makes sense to me that we could check for this cases of more than
one tracking slashes and convert them to only one, if it existed.
Created attachment 22769 [details]
Patch improved for fixing the crash.
This new patch is far safer, but I'm not sure if it will comply with all
standards. Basically, safer compared to the previous because in the case that
_str wasn't finishing in '/', I had trailingSlashesToRemove = -1, and it was
later being called with a count() - (-1), resulting in a bye-bye ranges.
It also prevents from crashing when calling "protocol:", by adding a '/'. This
is the part I'm not sure of taking care of standards...
Do you think this patch can be committed ? If so, please speak up.
Previous patches are wrong because they make some KUrl tests to fail. Working on a different solution. Created attachment 22775 [details]
Patch improved for fixing the crash
This is another way of proceeding for fixing this crash.
I really think David Faure should take a look here before taking any decision.
Can you explain the patch from comment #4? The traceback (by word) was: - void KDirModelPrivate::_k_slotNewItems(const KFileItemList& items) is called (we have new items) - const QPair<int, KDirModelNode*> result = nodeForUrl(dir); // O(n*m) - QList<KDirModelNode *>::const_iterator it = dirNode->m_childNodes.begin(); const QList<KDirModelNode *>::const_iterator end = dirNode->m_childNodes.end(); - it == end (empty list) => foundChild = false - return qMakePair(0, static_cast<KDirModelNode*>(0)); - assert at Q_ASSERT(result.second); // Are you calling KDirLister::openUrl(url,true,false)? Please use expandToUrl() instead. Hello gdb :-) The real reasoning behind this is "KDirModel can't put items under a root that has no path". So we need a path for the root, for instance by doing a redirect in the slave; or maybe we can change KDirModel to assume "/" as root path if the root path was empty --- and the slave hasn't redirected us elsewhere, like kio_ftp does. SVN commit 757313 by dfaure: Assume "/" when the URL has no path (and the slave didn't redirect) BUG: 154819 M +9 -2 kdirmodel.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=757313 |