DevHeads.net

Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)

See also,
<a href="http://bugs.kde.org/285028" title="http://bugs.kde.org/285028">http://bugs.kde.org/285028</a>
( and <a href="https://bugreports.qt.nokia.com/browse/QTBUG-22382" title="https://bugreports.qt.nokia.com/browse/QTBUG-22382">https://bugreports.qt.nokia.com/browse/QTBUG-22382</a> )

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

Comments

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/27/2011 - 15:11

On Thursday, 27 de October de 2011 13:32:51 Rex Dieter wrote:
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.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Milian Wolff at 10/27/2011 - 17:17

On Thursday 27 October 2011 21:11:11 Thiago Macieira wrote:
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...

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/27/2011 - 17:35

On Thursday, 27 de October de 2011 23:17:49 Milian Wolff wrote:
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.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Kevin Kofler at 10/28/2011 - 11:40

Thiago Macieira wrote:
It is what real-world users need in a network-transparent application!

They will want to enter a file name in most cases, but if they need to open
or save a file from/to the network, they have to enter a URL instead. If we
don't have a fromPathOrUrl API, we have to do what it does again and again
in every application to support what our users actually NEED. Users most
definitely DO NOT WANT to have to write file: in front of every file name
(the common case) (nor to have to encode all special characters in it), nor
is it acceptable to just forego network transparency.

This basic principle is also why there is no KFileRequester, only a
KUrlRequester (which supports a LocalOnly flag for when you really cannot
support KIO network transparency for some reason).

It is trivial to decide whether something is a file name or a URL: if
there's a scheme, it's a URL, otherwise, in this context, it's a file name.
Of course, a relative path COULD refer to a remote location in other
contexts, e.g. in links in web pages, where the context is the directory
containing the web page. But in the contexts we are discussing here, the
reference context is the local current directory, so relative paths are
relative to that and thus always local.

Now we can argue that applications should in fact be using fromPathToUrl on
those relative paths, then the new toLocalFile should also work for them.
But the problem is, they currently don't, and your incompatible change
breaks them.

Kevin Kofler

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By =?ISO-8859-1?Q?... at 10/28/2011 - 04:41

On 10/27/2011 11:35 PM, Thiago Macieira wrote:
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: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/28/2011 - 05:22

On Friday, 28 de October de 2011 10:41:36 Sebastian Trüg wrote:
Let me repeat: the problem already existed. This just made the broken code
more evident. You should fix it ANYWAY.

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.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Kevin Kofler at 10/28/2011 - 11:43

Thiago Macieira wrote:
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.

Kevin Kofler

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/28/2011 - 19:05

On Friday, 28 de October de 2011 17:43:32 Kevin Kofler wrote:
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.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Milian Wolff at 10/28/2011 - 22:38

On Saturday 29 October 2011 01:05:18 Thiago Macieira 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?

2) Is the -D... define to catch this problem at compile time already supported
in Qt 4.7?

I ask, because both KDevplatform and KDevelop seem to compile just fine with
the additional strict define...

thanks

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/29/2011 - 03:25

On Saturday, 29 de October de 2011 04:38:00 Milian Wolff wrote:
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.

Yes.

Note it just makes the constructor explicit: it's meant to catch accidental
conversions. If you spell out QUrl, it will still get used.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Kevin Kofler at 10/30/2011 - 13:45

Thiago Macieira wrote:
No, it doesn't.

There's actually a difference between KUrl(str):
<a href="https://projects.kde.org/projects/kde/kdelibs/repository/entry/kdecore/io/kurl.cpp?rev=KDE%2F4.7#L400" title="https://projects.kde.org/projects/kde/kdelibs/repository/entry/kdecore/io/kurl.cpp?rev=KDE%2F4.7#L400">https://projects.kde.org/projects/kde/kdelibs/repository/entry/kdecore/i...</a>
and KUrl::fromPathToUrl(str):
<a href="https://projects.kde.org/projects/kde/kdelibs/repository/entry/kdecore/io/kurl.cpp?rev=KDE%2F4.7#L1669" title="https://projects.kde.org/projects/kde/kdelibs/repository/entry/kdecore/io/kurl.cpp?rev=KDE%2F4.7#L1669">https://projects.kde.org/projects/kde/kdelibs/repository/entry/kdecore/i...</a>
and it affects relative paths. KUrl(str) will treat a relative path as a
URL, KUrl::fromPathToUrl(str) will treat it as a path! Both treat absolute
paths as paths and URLs with scheme as URLs.

So it looks like all code passing relative paths to KUrl should be using
fromPathToUrl instead. The problem is that fromPathToUrl is marked
deprecated! The rationale is:
* \deprecated
* Since KDE4 you can pass both urls and paths to the KUrl constructors.
* Use KUrl(text) instead.
but as I explained above, those are not at all the same thing! And I don't
think the KUrl constructor should be changed, since there are contexts such
as web browsers where you want relative paths to be URLs. So fromPathToUrl
needs to be undeprecated (the code costs almost nothing in terms of space)
and all functions constructing a KUrl from something which may be a local
relative path should be changed to use fromPathOrUrl.

An alternative solution would be to change the KUrl constructor to work like
fromPathToUrl and to introduce a static fromUrl function (to pair with
fromPath), which would be used in e.g. web browsers. But IMHO this is
source-incompatible and not suitable for 4.x. It might be the right thing to
do for KF5 though.

Kevin Kofler

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/30/2011 - 14:04

On Sunday, 30 de October de 2011 18:45:32 Kevin Kofler wrote:
Right, it doesn't call that function, but it does try to detect a path and set
appropriately.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Kevin Kofler at 10/30/2011 - 14:02

Kevin Kofler wrote:
Oops, I misread the code!

They both do the same thing after all, fromPathToUrl explicitly does NOT
treat relative paths as paths.

But that doesn't change much of the rest of my message: we definitely do
need an API which treats relative paths as file paths (for most
applications) and an API which treats relative paths as relative URLs (for
web browsers and such).

I think there should be 3 ways to construct a URL:
1. from a path (always), i.e. the current KUrl::fromPath
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
3. from a URL (always) – web browsers or any other app where the relative
context is a URL context should use this instead of 2.

Kevin Kofler

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/30/2011 - 14:36

On Sunday, 30 de October de 2011 19:02:48 Kevin Kofler wrote:
QUrl::fromLocalFile

I disagree and will not implement this in QUrl.

QUrl's constructor, QUrl::fromEncoded

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Kevin Kofler at 10/30/2011 - 17:34

Thiago Macieira 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
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 (an

By Thiago Macieira at 10/30/2011 - 19:27

On Sunday, 30 de October de 2011 22:34:48 Kevin Kofler wrote:
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.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Milian Wolff at 10/29/2011 - 08:43

On Saturday 29 October 2011 09:25:03 Thiago Macieira 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?

Thanks

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By David Faure at 10/31/2011 - 06:24

On Saturday 29 October 2011 14:43:20 Milian Wolff wrote:
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.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/30/2011 - 04:00

On Saturday, 29 de October de 2011 14:43:20 Milian Wolff wrote:
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).

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Rex Dieter at 10/28/2011 - 23:56

KUrl uses QUrl behind the scenes for several cases.

-- rex

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By David Jarvie at 10/28/2011 - 19:37

On Saturday 29 October 2011 00:05:18 Thiago Macieira wrote:
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.

Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl

By Albert Astals Cid at 10/28/2011 - 08:22

A Divendres, 28 d'octubre de 2011, Thiago Macieira vàreu escriure:
Right, it was wrong if it had to accept user input but for internal hardcoded
paths, it worked and now it does not.

Albert

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Thiago Macieira at 10/28/2011 - 09:18

On Friday, 28 de October de 2011 14:22:02 Albert Astals Cid wrote:
Right. If it is user input, we provide QUrl::fromuserInput, which implements
some heuristics.

But the case being discussed here was that the data was already
programmatically available, which meant that the heuristics should have been
applied already.

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Milian Wolff at 10/27/2011 - 18:41

On Thursday 27 October 2011 23:35:15 Thiago Macieira wrote:
<snip>

ok cool. I'll add this to kdevelop then. is this new in 4.8 or should it break
in 4.7 already if there are issues?

thanks

Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl

By Albert Astals Cid at 10/27/2011 - 16:31

A Dijous, 27 d'octubre de 2011, Thiago Macieira vàreu escriure:
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

Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl

By Kevin Kofler at 10/28/2011 - 11:30

Albert Astals Cid wrote:
+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 (an

By Christoph Feck at 10/28/2011 - 11:53

On Friday 28 October 2011 17:30:44 Kevin Kofler wrote:
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 (an

By Thiago Macieira at 10/27/2011 - 16:37

On Thursday, 27 de October de 2011 22:31:15 Albert Astals Cid wrote:
Right. So we should just not release updates, because fixing bugs changes
behaviour.

Good thinking. Call me when you get to make such decisions.

Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl

By Albert Astals Cid at 10/27/2011 - 16:53

A Dijous, 27 d'octubre de 2011, Thiago Macieira vàreu escriure:
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

Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (an

By Rex Dieter at 10/27/2011 - 15:22

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