[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Colin J Thomson via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

Colin J Thomson  changed:

   What|Removed |Added

 CC|colin.thom...@g6avk.co.uk   |

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Franky via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #22 from Franky  ---
Thanks :-)

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Rex Dieter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

Rex Dieter  changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |FIXED

--- Comment #21 from Rex Dieter  ---
It was fixed in both knotifications and knotifyconfig after 5.22.0 release. 
The fix will be included in kf5 5.23.0 release.

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Franky via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #20 from Franky  ---
With which version should this be fixed? Here on arch-linux it is working only
when i use the "file://" prefix. 
Installed Versions:

knotifications 5.22.0-1 (kf5)
knotifyconfig 5.22.0-1 (kf5)
phonon-qt4 4.9.0-1

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Colin J Thomson via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #19 from Colin J Thomson  ---
With the new fixes all works fine for me - F24

kf5-knotifications-5.21.0-3.fc24.x86_64
kf5-knotifyconfig-5.21.0-2.fc24.x86_64
phonon-4.9.0-3.fc24.x86_64

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Harald Sitter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #17 from Harald Sitter  ---
Git commit 4832f7d9f2f3bd0fa8ab9b9162bf50e855efc448 by Harald Sitter.
Committed on 09/05/2016 at 07:27.
Pushed by sitter into branch 'master'.

use QUrl::fromUserInput to construct sound url

QUrl() would treats it as a parsable uri, but they aren't e.g. # in a uri
separates segments in a local file path it simply is a #.
This "accidentally" worked in Phonon < 4.9 as Phonon obtained string
representations in a way that would bypass internal QUrl checks for
fileyness and URI ambiguity. Since 4.9 Phonon expects scheme-less URLs to
be local files, but QUrl() would most of the time not do that since it
would honestly think the soundfilename is a random (i.e. not necessarily
local) uri.

To fix this use QUrl::fromUserInput which behaves exactly like what we
need to properly resolve relative names, urls, paths, full URIs.

This now works with input of the type:
- Oxygen-Sys-Special.ogg
- /usr/share/sounds/Oxygen-Sys-Special.ogg
- file:///usr/share/sounds/Oxygen-Sys-Special.ogg
- /usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- file:///usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- http://people.ubuntu.com/~apachelogger/sounds/sounds-3.5/KDE_Glass_Break.ogg
(yes, we can have http notifications https://xkcd.com/1172/)
REVIEW: 127829

M  +13   -8src/knotifyconfigactionswidget.cpp

http://commits.kde.org/knotifyconfig/4832f7d9f2f3bd0fa8ab9b9162bf50e855efc448

--- Comment #18 from Harald Sitter  ---
Git commit 9db06adc8114163f401417064b07772139bc36bc by Harald Sitter.
Committed on 09/05/2016 at 07:27.
Pushed by sitter into branch 'master'.

use QUrl::fromUserInput to construct sound url

QUrl() would treats it as a parsable uri, but they aren't e.g. # in a uri
separates segments in a local file path it simply is a #.
This "accidentally" worked in Phonon < 4.9 as Phonon obtained string
representations in a way that would bypass internal QUrl checks for
fileyness and URI ambiguity. Since 4.9 Phonon expects scheme-less URLs to
be local files, but QUrl() would most of the time not do that since it
would honestly think the soundfilename is a random (i.e. not necessarily
local) uri.

To fix this use QUrl::fromUserInput which behaves exactly like what we
need to properly resolve relative names, urls, paths, full URIs.

This now works with input of the type:
- Oxygen-Sys-Special.ogg
- /usr/share/sounds/Oxygen-Sys-Special.ogg
- file:///usr/share/sounds/Oxygen-Sys-Special.ogg
- /usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- file:///usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- http://people.ubuntu.com/~apachelogger/sounds/sounds-3.5/KDE_Glass_Break.ogg
(yes, we can have http notifications https://xkcd.com/1172/)
REVIEW: 127830

M  +16   -9src/notifybyaudio.cpp

http://commits.kde.org/knotifications/9db06adc8114163f401417064b07772139bc36bc

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Harald Sitter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #17 from Harald Sitter  ---
Git commit 4832f7d9f2f3bd0fa8ab9b9162bf50e855efc448 by Harald Sitter.
Committed on 09/05/2016 at 07:27.
Pushed by sitter into branch 'master'.

use QUrl::fromUserInput to construct sound url

QUrl() would treats it as a parsable uri, but they aren't e.g. # in a uri
separates segments in a local file path it simply is a #.
This "accidentally" worked in Phonon < 4.9 as Phonon obtained string
representations in a way that would bypass internal QUrl checks for
fileyness and URI ambiguity. Since 4.9 Phonon expects scheme-less URLs to
be local files, but QUrl() would most of the time not do that since it
would honestly think the soundfilename is a random (i.e. not necessarily
local) uri.

To fix this use QUrl::fromUserInput which behaves exactly like what we
need to properly resolve relative names, urls, paths, full URIs.

This now works with input of the type:
- Oxygen-Sys-Special.ogg
- /usr/share/sounds/Oxygen-Sys-Special.ogg
- file:///usr/share/sounds/Oxygen-Sys-Special.ogg
- /usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- file:///usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- http://people.ubuntu.com/~apachelogger/sounds/sounds-3.5/KDE_Glass_Break.ogg
(yes, we can have http notifications https://xkcd.com/1172/)
REVIEW: 127829

M  +13   -8src/knotifyconfigactionswidget.cpp

http://commits.kde.org/knotifyconfig/4832f7d9f2f3bd0fa8ab9b9162bf50e855efc448

--- Comment #18 from Harald Sitter  ---
Git commit 9db06adc8114163f401417064b07772139bc36bc by Harald Sitter.
Committed on 09/05/2016 at 07:27.
Pushed by sitter into branch 'master'.

use QUrl::fromUserInput to construct sound url

QUrl() would treats it as a parsable uri, but they aren't e.g. # in a uri
separates segments in a local file path it simply is a #.
This "accidentally" worked in Phonon < 4.9 as Phonon obtained string
representations in a way that would bypass internal QUrl checks for
fileyness and URI ambiguity. Since 4.9 Phonon expects scheme-less URLs to
be local files, but QUrl() would most of the time not do that since it
would honestly think the soundfilename is a random (i.e. not necessarily
local) uri.

To fix this use QUrl::fromUserInput which behaves exactly like what we
need to properly resolve relative names, urls, paths, full URIs.

This now works with input of the type:
- Oxygen-Sys-Special.ogg
- /usr/share/sounds/Oxygen-Sys-Special.ogg
- file:///usr/share/sounds/Oxygen-Sys-Special.ogg
- /usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- file:///usr/share/sounds/#KDE-Im-Cant-#Connect.ogg
- http://people.ubuntu.com/~apachelogger/sounds/sounds-3.5/KDE_Glass_Break.ogg
(yes, we can have http notifications https://xkcd.com/1172/)
REVIEW: 127830

M  +16   -9src/notifybyaudio.cpp

http://commits.kde.org/knotifications/9db06adc8114163f401417064b07772139bc36bc

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Rex Dieter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #16 from Rex Dieter  ---
Did test notifications from 'kcmshell4 kcmnotify' both relative and full path
audio is fine, so nevermind

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Rex Dieter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #15 from Rex Dieter  ---
See also:
knotifyconfig : https://git.reviewboard.kde.org/r/127829/
knotifications: https://git.reviewboard.kde.org/r/127830

Harald, may related kde4 components (like kdelibs/libknotifyconfig) need fixing
too?

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Orbmiser via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #14 from Orbmiser  ---
(In reply to Franky from comment #13)
> Do i understand it right, the main problem is compatibility with qt4?

Nope recent breakage worked fine last month but updates last couple of weeks
broke it.

KDE Plasma 5.6.3
KDE Frameworks 5.21.0
Qt version: 5.6.0

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Franky via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #13 from Franky  ---
Do i understand it right, the main problem is compatibility with qt4?

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Harald Sitter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

Harald Sitter  changed:

   What|Removed |Added

 CC||dvra...@kde.org,
   ||mklape...@kde.org

--- Comment #12 from Harald Sitter  ---
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.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Colin J Thomson via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #11 from Colin J Thomson  ---
Fixes it for me also - F23

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Rex Dieter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

--- Comment #10 from Rex Dieter  ---
I think I've narrowed it down to the commit that fixes bug #356218 as the
culprit behind this apparent regression:

http://commits.kde.org/phonon/621ffb823d45a88a02602fb4ee4b769dfbc0c16a

Reverting that fixes issues as reported here for me

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Colin J Thomson via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

Colin J Thomson  changed:

   What|Removed |Added

 CC||colin.thom...@g6avk.co.uk

-- 
You are receiving this mail because:
You are watching all bug changes.

[Phonon] [Bug 337276] fully-qualified audio notifications no longer play

[Rex Dieter via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=337276

Rex Dieter  changed:

   What|Removed |Added

Version|5.20.0  |unspecified
 CC||myr...@kde.org,
   ||romain.per...@gmail.com
Product|frameworks-knotifications   |Phonon
   Assignee|mklape...@kde.org   |sit...@kde.org
  Component|general |general
Summary|Sound will not play when|fully-qualified audio
   |Notification even fired |notifications no longer
   ||play

--- Comment #9 from Rex Dieter  ---
OK, starting to think at least part of what's going on here is a phonon
regression from 4.8 => 4.9

With phonon-4.8 audio preview of /usr/share/sounds/KDE-Im-Cant-Connect.ogg
works, after upgrading to phonon-4.9 it doesn't

-- 
You are receiving this mail because:
You are watching all bug changes.