https://bugs.kde.org/show_bug.cgi?id=337276
Harald Sitter <sit...@kde.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dvra...@kde.org,
| |mklape...@kde.org
--- Comment #12 from Harald Sitter <sit...@kde.org> ---
Ok, so. This is somewhat tricky business and I'd love some input on how to deal
with it. Lot's of technobabble upcoming.
This was originally changed because of bug 356218 where having a '#' in a file
name would fail to be encoded correctly because QUrl upon construction would
internally safe-encode a path, so internally 'Foo#.mp4' is 'Foo%23.mp4'.
QUrl.toString would then output the internal variant rather than fully decoded
back to 'Foo#.mp4' which clashes with the encoding Phonon's Mrl applies later
on. Resulting in 'Foo%2523.mp4' as output (that is literally a percent encoded
percent character followed by 23).
So it was changed to toLocalString, since that always fully decodes.
The problem we are facing in this bug here is that toLocalString only outputs
anything if the string is actually known to be local, which it isn't because it
was constructed via QUrl() and didn't have a file:// scheme so it could be cat
for all QUrl knows.
I am proposing that the bug is in fact in KNotifications and not in Phonon.
KNotifications in the case of an absolute path does the following:
> QUrl soundURL = QUrl(soundFilename); // this CTOR accepts both absolute
> paths (/usr/share/sounds/blabla.ogg and blabla.ogg) w/o screwing the scheme
which looks innocent enough, except it is not. What this does is it assumes
soundFilename is a URL, which is not wrong, it is however not a parsable URL.
The key difference is that in a parsable URL #?&= have meaning. In one that
isn't, they don't. So, what that means is that if soundFilename is '/Foo#.mp3'
this results in parsing leading to the path() being '/Foo' and the fragment()
being '#.mp3', so unless Phonon were to manually concatenate all parts of the
URL (scheme, host, path, query, fragment) in their fully decoded form we cannot
get to the actual path. This is in particular true since we cannot
toString(FullyDecode) because that would be ambiguous precisely because of
cases like above where the path may contain a # but then the URL could also
have a fragment.
The reason it worked previously is precisely because of bug 356218. As long as
a local file path doesn't contain a reserved character all is fine since we'd
always correctly decode with toString() and then apply our encoding rules on
top. If the file path contained such a character it would still work for QUrl()
constructed *parsed* paths (i.e. where for example #.mp3 is a fragment
internally) but would then fail for QUrl::fromUserInput or QUrl::fromLocalFile
as there toString would return the half-encoded internal variant to prevent
ambiguity.
So. Can we fix this? Absolutely! We could toEncoded, then run that through the
static fromPercentEncoding and should get the fully decoded string again.
BUT!
I'm going out on a limb here and say that the breakage in 4.9.0, while very
unfortunate short-term, is a good thing. The way QUrl() is used in
KNotifications is quite simply wrong. KNotifications should use fromLocalFile
or fromUserInput (latter actually can replace the entire file name code there).
The fact that it worked by "accident" earlier doesn't help make it behavior I
would want to hold on to in Phonon.
On the other hand I do also dislike compatibility break, so I am torn on this.
Anyone got thoughts?
A middle-ground would perhaps be to reach for maximum compatibility when
building with Qt4 and leave the breakage for Qt5 software using Qt4 probably
won't be getting an update to adjust QUrl usage if needed.
--
You are receiving this mail because:
You are watching all bug changes.
