Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Saturday 29 October 2011 14:43:20 Milian Wolff wrote: On Saturday 29 October 2011 09:25:03 Thiago Macieira wrote: On Saturday, 29 de October de 2011 04:38:00 Milian Wolff wrote: 1) When does it manifest? Apparently when using QUrl(...) directly, if I'm not mistaken. But what if we use KUrl? You're correct: this problem manifests when you use QUrl's constructor directly, assuming it will understand a local file path for what it is and not parse it is a URL. KUrl's constructor calls fromPathOrUrl, so it will try to guess according to some heuristics. The reason why this behaviour exists in QUrl and why I think KUrl is flawed is that there's a category of URLs that are incomplete, the relative URLs, also known as URL refs. That's what you see in the href or src attributes in HTML: those are real URL components, but they are partial. They need to be resolved against a base URL, usually the document's. There's no way to tell apart a local file path from a URL ref and the examples I gave show how it would be interpreted differently. Right, but you should agree that relative remote adresses can only occur in a browser context. At least in KDevelop + Kate I don't see any way for a user to provide a relative url, so I hope that the existing codebase will work just fine with Qt 4.8. Anyone tried it already maybe? Exactly, most of the KDE code does not use relative paths/urls, which is why not much code is affected by this change, and also why KUrl(QString) is maybe a bit inconsistent / not well thought out when it comes to relative paths. I admit I didn't think much of that use case, I was merely trying to provide full path or URL detection, which I believe is the most needed use case, and does NOT have the bugs Thiago mentionned (/tmp/Who let the dogs out?.mp3 and /tmp/Mambo #5.mp3 work just fine). KUrl uses QUrl, but the API is different, and this doesn't trigger these issues. The handling of relative URLs would need more thought and unit-testing (so that such breakages don't go unnoticed), but personally I don't like using KUrl(QString) for these, and apparently Thiago neither. As to what to do now, to restore behavior compatibility, I'm not sure. I don't even see how the knotify use case worked, because indeed KUrl was parsing as a url something that was in fact a relative path. KUrl isn't meant to store a relative location, and KUrlRequester always returns an absolute url when using the file dialog, one would have to type something blindly to get a relative filename in there, no? Anyway KUrlRequester should be fixed if it returns relative paths, I don't think it should let apps resolve that to a full path when it can do so itself. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Saturday, 29 de October de 2011 14:43:20 Milian Wolff wrote: Right, but you should agree that relative remote adresses can only occur in a browser context. At least in KDevelop + Kate I don't see any way for a user to provide a relative url, so I hope that the existing codebase will work just fine with Qt 4.8. Anyone tried it already maybe? I've been running KDE with Qt 4.8 for over 6 months. The only issue I see is a font one: a variable-width font is selected in some applications when a fixed- width one was intended. It used to happen with kate/kwrite as well as Qt Creator but it has disappeared from there -- now it only appears in WebKit (rekonq konqueror). -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Sunday, 30 de October de 2011 18:45:32 Kevin Kofler wrote: not parse it is a URL. KUrl's constructor calls fromPathOrUrl, so it will try to guess according to some heuristics. No, it doesn't. Right, it doesn't call that function, but it does try to detect a path and set appropriately. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
Thiago Macieira wrote: 2. heuristically (replacing the current KUrl::KUrl and KUrl::fromPathToUrl): * absolute paths should be file paths * relative paths should be file paths (!) (currently, they're URLs) * everything with a URL scheme (protocol) should be a URL I disagree and will not implement this in QUrl. You keep claiming this is not needed or useful, but how else would you suggest handling the use case of a network-transparent file dialog (or file requester, i.e. a line edit + an open button which brings up a file dialog, to be used in dialogs, see KUrlRequester) where the USER expects to be able to type in any of: * an absolute path, * a path relative to some local current directory or * a URL, which will always be absolute? Should all the code needing to handle something like this come with their own heuristics to handle this same user input? Anyway, it can be implemented in KUrl (replacing or adding to the current heuristics, which are also not in QUrl). Kevin Kofler
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Sunday, 30 de October de 2011 22:34:48 Kevin Kofler wrote: You keep claiming this is not needed or useful, but how else would you suggest handling the use case of a network-transparent file dialog (or file That's QUrl::fromUserInput, which makes no claim to what it considers. It's just a heuristic algorithm we are free to change at will. As its name implies, it should not be used for anything but user input. Sources of parsed URL should use the proper functions and provide consistent input, not guesses. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On 10/27/2011 11:35 PM, Thiago Macieira wrote: On Thursday, 27 de October de 2011 23:17:49 Milian Wolff wrote: On Thursday 27 October 2011 21:11:11 Thiago Macieira wrote: On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote: See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? As we discussed on IRC, any string source must be properly labelled whether they are a URL or they are a local file. They cannot be both. is there at least a qWarning emitted in such a case, so we can find these problems with QT_FATAL_WARNINGS=1 ? No, but there's something better: #define QURL_NO_CAST_FROM_QSTRING Your code will not compile when you're auto-casting. You'll need to select what to do: QUrl::fromEncoded() - if it's a URL, with the file scheme QUrl::fromLocalFile() - if it's a file name This option remains in Qt 5, but there there's going to be a new method to convert from QString to QUrl without passing through QByteArray. This is extremely important to get right because a filename and a URL are NOT the same. Certain characters in the string are interpreted differently depending on what the string is: %, # and ? in particular. So, to be honest, the bug *already* *existed* in your code if you're finding these problems now. I just made it blatantly clear, and it's been there for a year for people to see. I'm also calling right now KUrl's fromPathOrUrl a fatally flawed design. Be that as it may but the fact remains that KDE potentially does not work with Qt 4.8. Is that really a risk worth taking? I am all for fixed semantics in the methods but this seems like a problematic case. Cheers Sebastian
Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
A Divendres, 28 d'octubre de 2011, Thiago Macieira vàreu escriure: On Friday, 28 de October de 2011 10:41:36 Sebastian Trüg wrote: So, to be honest, the bug already existed in your code if you're finding these problems now. I just made it blatantly clear, and it's been there for a year for people to see. I'm also calling right now KUrl's fromPathOrUrl a fatally flawed design. Be that as it may but the fact remains that KDE potentially does not work with Qt 4.8. Is that really a risk worth taking? I am all for fixed semantics in the methods but this seems like a problematic case. Let me repeat: the problem already existed. This just made the broken code more evident. You should fix it ANYWAY. Right, it was wrong if it had to accept user input but for internal hardcoded paths, it worked and now it does not. Albert If you had a file path of: /tmp/Who let the dogs out?.mp3 or/tmp/Mambo #5.mp3 In Qt 4.7, toLocalFile on those URLs above would return: /tmp/Who let the dogs out and /tmp/Mambo Which is quite wrong already. From Qt 4.8 on, this returns empty in all cases, showing that you parsed the URL wrongly. It should be easier to spot where you made the mistake because you don't have to use specially-crafted filenames. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
Albert Astals Cid wrote: Personally i find it another joke in the history of Qt, saying you maintain API and ABI (that you do) but then making functions behave totally different from one version to another is just plain useless. +1 You just CANNOT change the behavior of an existing function in such a way. Kevin Kofler
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Friday 28 October 2011 17:30:44 Kevin Kofler wrote: Albert Astals Cid wrote: Personally i find it another joke in the history of Qt, saying you maintain API and ABI (that you do) but then making functions behave totally different from one version to another is just plain useless. +1 You just CANNOT change the behavior of an existing function in such a way. Kevin Kofler Give me a break. I am running KDE/master with Qt 4.8 branch since some time, and if this change would cause bugs to show up everwhere the toLocalFile() method is used, then I certainly had noticed this change. From the discussion so far it looks like only KNotify is affected, and I am sure KNotify can be fixed to adapt to the Qt 4.8 change. If other issues surface, they will be noticed, because an empty file name certainly cannot work at all. Bugs because of missing characters in file names (as Thiago pointed out) are harder to see. Additionally, this certainly is not the first time an update to Qt caused regression we had to fix. Starting from source-incompatible changes and ending with issues with the raster graphics system - we went through all of them; and for the better. If we insist bug-for-bug compatibility in Qt for the next years, I am sure Qt developers will happily stop fixing any bugs. Christoph Feck (kdepepo) KDE Quality Team
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Friday, 28 de October de 2011 17:43:32 Kevin Kofler wrote: Thiago Macieira wrote: Which is quite wrong already. From Qt 4.8 on, this returns empty in all cases, showing that you parsed the URL wrongly. It should be easier to spot where you made the mistake because you don't have to use specially-crafted filenames. Such a change might make sense for 5.0, but not for 4.8! Or if you really want to add the changed API to 4.x, you have to do it with a new name, deprecating (but not removing!) toLocalFile. Let's just say I disagree. This change was for 4.7, but it was reverted and added to 4.8, giving you well over ONE YEAR to adapt. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Saturday 29 October 2011 00:05:18 Thiago Macieira wrote: On Friday, 28 de October de 2011 17:43:32 Kevin Kofler wrote: Thiago Macieira wrote: Which is quite wrong already. From Qt 4.8 on, this returns empty in all cases, showing that you parsed the URL wrongly. It should be easier to spot where you made the mistake because you don't have to use specially-crafted filenames. Such a change might make sense for 5.0, but not for 4.8! Or if you really want to add the changed API to 4.x, you have to do it with a new name, deprecating (but not removing!) toLocalFile. Let's just say I disagree. This change was for 4.7, but it was reverted and added to 4.8, giving you well over ONE YEAR to adapt. Only for those who knew about it a year ago. Many people will probably only find out when things don't work with 4.8. -- David Jarvie. KDE developer. KAlarm author -- http://www.astrojar.org.uk/kalarm
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
Milian Wolff wrote: Could someone maybe explain a few points on this issue for me? 1) When does it manifest? Apparently when using QUrl(...) directly, if I'm not mistaken. But what if we use KUrl? KUrl uses QUrl behind the scenes for several cases. -- rex
Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? -- rex
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote: See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? As we discussed on IRC, any string source must be properly labelled whether they are a URL or they are a local file. They cannot be both. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
Thiago Macieira wrote: On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote: See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? As we discussed on IRC, any string source must be properly labelled whether they are a URL or they are a local file. They cannot be both. I understand that (and I agree, fwiw). However, KDE has seemingly many cases where this is not followed wrt string source, knotify audio event sources (in notification defaults or even users' existing config files) and kynotifyconfigactionswidget.cpp's use of KUrlRequester class are examples. -- rex
Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
A Dijous, 27 d'octubre de 2011, Thiago Macieira vàreu escriure: On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote: See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? As we discussed on IRC, any string source must be properly labelled whether they are a URL or they are a local file. They cannot be both. Personally i find it another joke in the history of Qt, saying you maintain API and ABI (that you do) but then making functions behave totally different from one version to another is just plain useless. Albert -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Thursday, 27 de October de 2011 22:31:15 Albert Astals Cid wrote: Personally i find it another joke in the history of Qt, saying you maintain API and ABI (that you do) but then making functions behave totally different from one version to another is just plain useless. Right. So we should just not release updates, because fixing bugs changes behaviour. Good thinking. Call me when you get to make such decisions. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
A Dijous, 27 d'octubre de 2011, Thiago Macieira vàreu escriure: On Thursday, 27 de October de 2011 22:31:15 Albert Astals Cid wrote: Personally i find it another joke in the history of Qt, saying you maintain API and ABI (that you do) but then making functions behave totally different from one version to another is just plain useless. Right. So we should just not release updates, because fixing bugs changes behaviour. This is not what i meant, and you know it. But as it is obvious you do not want to have a constructive discussion, let's end it here. Albert Good thinking. Call me when you get to make such decisions. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Thursday 27 October 2011 21:11:11 Thiago Macieira wrote: On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote: See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? As we discussed on IRC, any string source must be properly labelled whether they are a URL or they are a local file. They cannot be both. is there at least a qWarning emitted in such a case, so we can find these problems with QT_FATAL_WARNINGS=1 ? bye, Milian - who fears lots of issues in the huge KDevelop codebase... -- Milian Wolff m...@milianw.de http://milianw.de
Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)
On Thursday, 27 de October de 2011 23:17:49 Milian Wolff wrote: On Thursday 27 October 2011 21:11:11 Thiago Macieira wrote: On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote: See also, http://bugs.kde.org/285028 ( and https://bugreports.qt.nokia.com/browse/QTBUG-22382 ) In Qt 4.8, QUrl.toLocalFile now seems, by design, to return NULL for urls lacking any scheme. Discovered this the hard way figuring out why all my audio knotifications were quiet. Since audio event sources are simple filenames, e.g. KDE-K3B-Finish-Success.ogg, and QString soundFile = soundFileURL.toLocalFile(); no longer works. Any suggestions or advice on how best to deal with this? As we discussed on IRC, any string source must be properly labelled whether they are a URL or they are a local file. They cannot be both. is there at least a qWarning emitted in such a case, so we can find these problems with QT_FATAL_WARNINGS=1 ? No, but there's something better: #define QURL_NO_CAST_FROM_QSTRING Your code will not compile when you're auto-casting. You'll need to select what to do: QUrl::fromEncoded() - if it's a URL, with the file scheme QUrl::fromLocalFile() - if it's a file name This option remains in Qt 5, but there there's going to be a new method to convert from QString to QUrl without passing through QByteArray. This is extremely important to get right because a filename and a URL are NOT the same. Certain characters in the string are interpreted differently depending on what the string is: %, # and ? in particular. So, to be honest, the bug *already* *existed* in your code if you're finding these problems now. I just made it blatantly clear, and it's been there for a year for people to see. I'm also calling right now KUrl's fromPathOrUrl a fatally flawed design. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part.
