DevHeads.net

Review Request: Hacky workaround for https://bugs.kde.org/show_bug.cgi?id=228005

Review request for KDE Runtime.

Description
<a href="https://bugs.kde.org/show_bug.cgi?id=228005" title="https://bugs.kde.org/show_bug.cgi?id=228005">https://bugs.kde.org/show_bug.cgi?id=228005</a>
if sound is not working for some reason (e.g. no phonon
backends are installed) the closed() signal never happens
and logoutSoundFinished() never gets called. This hack
makes sure that logout still occurs. Of course, on systems
with working sound there's a race.

Diffs
ksmserver/shutdown.cpp 7fd1e11

Diff: <a href="http://git.reviewboard.kde.org/r/103593/diff/diff" title="http://git.reviewboard.kde.org/r/103593/diff/diff">http://git.reviewboard.kde.org/r/103593/diff/diff</a>

Testing

Thanks,

Peter OGorman

Comments

Re: Review Request: Hacky workaround for https://bugs.kde.org/sh

By Commit Hook at 02/27/2012 - 18:39

This review has been submitted with commit ad188bfdb06af50448acbbde00556ebc3d29f418 by Lamarque V. Souza to branch master.

- Commit Hook

On Dec. 31, 2011, 3:16 a.m., Peter OGorman wrote:

Re: Review Request: Hacky workaround for https://bugs.kde.org/sh

By Commit Hook at 02/27/2012 - 18:39

This review has been submitted with commit 82d5f2a118a9a40e18b86628a3dc3109adf4f7bc by Lamarque V. Souza to branch KDE/4.8.

- Commit Hook

On Dec. 31, 2011, 3:16 a.m., Peter OGorman wrote:

Re: Review Request: Hacky workaround for https://bugs.kde.org/sh

By Dirk Tilger at 01/16/2012 - 13:15

While it definitely makes sense to make the KDE logout work, isn't the root of the problem that KNotification doesn't call the slot if no sound is available? Other applications would be affected by this very issue, if they are triggering sounds the same way. In my opinion it'd make more sense to fix KNotification to issue a ignore() slot call when no sound is available.
It wouldn't hurt, of course, to fix it in both places. ;)

- Dirk Tilger

On Dec. 31, 2011, 3:16 a.m., Peter OGorman wrote:

Re: Review Request: Hacky workaround for https://bugs.kde.org/sh

By Lamarque V. Souza at 01/04/2012 - 18:22

ksmserver/shutdown.cpp
<http://git.reviewboard.kde.org/r/103593/#comment7815>

As long as ksmserver is not multi-threaded the race will not cause problems because logoutSoundFinished slot returns if state is different from WaitingForKNotify and the last thing logoutSoundFinished slot does is call startKilling, which changes the state. The second logoutSoundFinished signal will test the state in logoutSoundFinished slot and return.

- Lamarque Vieira Souza

On Dec. 31, 2011, 3:16 a.m., Peter OGorman wrote:

Re: Review Request: Hacky workaround for https://bugs.kde.org/sh

By Peter OGorman at 01/07/2012 - 23:37

Done, thanks.

- Peter

On Dec. 31, 2011, 3:16 a.m., Peter OGorman wrote:

Re: Review Request: Hacky workaround for https://bugs.kde.org/sh

By Lamarque V. Souza at 01/07/2012 - 18:37

Please update the request header: add a small description in the summary instead of an url. Add the bugs that this patch fixes in the "Bugs" field, add bug numbers only, not the url. Bug #290625 "Shutdown does not work on Meego PA2" is also fixed by this patch.

If nobody comes up with a better solution I vote for shipping this request.

- Lamarque Vieira

On Dec. 31, 2011, 3:16 a.m., Peter OGorman wrote: