rcaelers/workrave — pull request #19 by jay (2012-06-05 06:09:22+00:00)
Title: Fix sound theme defaults for new users

I'm not sure if this is the best way to fix this. I think it's pretty good as long as it doesn't break anything else. In Windows it tests fine. It looks like the initial empty filenames for new users are caused because W32DirectSoundPlayer::get_sound_wav_file() always returns true and returns a wav_file of "".

>	workrave.exe!W32DirectSoundPlayer::get_sound_wav_file(SoundEvent snd, std::basic_string,std::allocator > & wav_file) Line 226	C++
workrave.exe!SoundPlayer::sync_settings() Line 480 + 0x22 bytes	C++
workrave.exe!SoundPlayer::register_sound_events(std::basic_string,std::allocator > theme) Line 397	C++
workrave.exe!SoundPlayer::init() Line 379	C++
workrave.exe!GUI::init_sound_player() Line 907	C++
workrave.exe!GUI::main() Line 223	C++
workrave.exe!run(int argc, char * * argv) Line 66	C++
workrave.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * szCmdLine, int iCmdShow) Line 108 + 0xb bytes	C++
workrave.exe!__tmainCRTStartup() Line 547 + 0x2c bytes	C
workrave.exe!WinMainCRTStartup() Line 371	C
kernel32.dll!@BaseThreadInitThunk@12() + 0x12 bytes
ntdll.dll!___RtlUserThreadStart@8() + 0x27 bytes
ntdll.dll!__RtlUserThreadStart@8() + 0x1b bytes

So during register there's a sync and driver->get_sound_wav_file() is called which is W32DirectSoundPlayer::get_sound_wav_file() and always returns true even if the key does not exist. So W32DirectSoundPlayer::get_sound_wav_file() returns true and wav_file = "", and that empty filename is then valid and set in the configuration.

And here's my fix, detailed in the commit message below:

frontend/common/src/SoundPlayer.cc

- (SoundPlayer::activate_theme)
Do not check that the filename is not empty before testing it. An empty
filename will now fail validation the same as if the filename does not
exist.

Prior to this change a new user would not receive the default settings
of a sound profile. Here's what happened with the break prelude sound,
for example:

A new user doesn't have the following keys:
HKEY_CURRENT_USER\AppEvents\Schemes\Apps\Workrave-crash\WorkraveBreakPrelude\.current
HKEY_CURRENT_USER\AppEvents\Schemes\Apps\Workrave-crash\WorkraveBreakPrelude\.default

So Workrave sets
HKEY_CURRENT_USER\Software\Workrave\sound\events\break_prelude as ""

Later activate_theme() is called by register_sound_events() and it
tests if break_prelude exists, and because it does it will not attempt
to set the filename. The filename, empty as "", will not fail
validation if it's empty.

As far as I know this was only a problem on Windows.