Comment 36 for bug 410466

Revision history for this message
In , Leo Milano (lmilano) wrote :

Hi Daniel

I agree, this has to be fixed anyway.

> There is still the bug in KNotify where it will load and then attempt to play a
> sound file even when it never initialised a player because NoSound is set.

Are you sure about this? I am looking at the function (maybe it was recently fixed). It doesn't seem to be attempting to _play_ it if the Private object is set to NoSound. It is actually doing this [1]:

00235 if(d->playerMode == Private::UsePhonon)
00236 {
00237 Player *player = d->playerPool.getPlayer();
00238 connect(player->media, SIGNAL(finished()), d->signalmapper, SLOT(map()));
00239 d->signalmapper->setMapping(player->media, eventId);
00240 player->play(soundFile);
00241 d->playerObjects.insert(eventId, player);
00242 }
00243 else if (d->playerMode == Private::ExternalPlayer && !d->externalPlayer.isEmpty())
00244 {
00245 // use an external player to play the sound
00246 KProcess *proc = new KProcess( this );
00247 connect( proc, SIGNAL(finished(int, QProcess::ExitStatus)),
00248 d->signalmapper, SLOT(map()) );
00249 d->signalmapper->setMapping( proc , eventId );
00250
00251 (*proc) << d->externalPlayer << soundFile;
00252 proc->start();
00253 }

So, in our case, both if() conditions will be false, and we'll just exit the function. However, we are not calling finish( eventId ) , which seems mandatory for other processes calling us to know _we_ are done [2]. So, the clean way to do this is to include it in the same if, just add one more condition (basically, the code you suggested above, but in line 254, in an else if() condition.

Cheers,
-- Leo

[1] http://api.kde.org/4.x-api/kdebase-runtime-apidocs/knotify/html/notifybysound_8cpp_source.html
[2] http://api.kde.org/4.x-api/kdebase-runtime-apidocs/knotify/html/classKNotifyPlugin.html#aac39c9af1599b8b4c60406eb8145d08c