The idea came up to remove all traces of System::Beep, which occurs randomly in the codebase. To get the occurrences: git grep -5 Sound::Beep Goal would be to remove it all. Also see: http://lists.freedesktop.org/archives/libreoffice-ux-advise/2012-February/000888.html
Ok, good idea, but "easyhack" should be ini the whiteboard....
Created attachment 64387 [details] Patch for fdo#48549 System::Beep() removal
I've written a patch that remove all call to Sound::Beep. There is still some warnings about unused parameters but I didn't wanted to change function prototypes. I'm new to LibreOffice codebase so please give me some feedback.
Hi Meko, nice work! Looks good to me. Here are some unused parameters: vcl/source/window/menu.cxx: The definition of nKeyCode is now unused (above your removal). So this else branch could go. basctl/source/basicide/baside2.cxx: sal_Bool bRemoved = pLayout->GetWatchWindow().RemoveSelectedWatch(); only call for pLayout->GetWatchWindow().RemoveSelectedWatch(); without saving it, as bRemoved would be unused. A question comes in mind: At some points the beep should say "here's an error". I've seen at least two points, where a debug message with OSL_FAIL() followed (sc/source/core/data/documen4.cxx). I think we should convert the beeps to debug messages at such points. Meko, what do you think? Maybe a post to the mailing list would be a good idea. Thomas
Created attachment 64453 [details] Patch for fdo#48549 System::Beep() removal v2
Hi, I've sent a new patch where I've removed the the two unused parameters mentioned by Thomas.
Mathieu Vonlanthen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=0f6101cfef4c2e45d9f1f1b3a61ef94799e4526b fdo#48549 System::Beep() removal
Meko, there are some more beeps (you get them with `git grep -5 Sound::Beep`): http://opengrok.libreoffice.org/xref/core/vcl/source/control/scrbar.cxx#988 In files were the Beep calls are gone, `#include <vcl/sound.hxx>` could go too. Also there are approx. three calls in binfilter. If you want do do some more cleanup feel free to do so. Finally there should be no calls to Beep() and you could remove the Beep virtuals and implementations.
Comment on attachment 64453 [details] Patch for fdo#48549 System::Beep() removal v2 2nd patch looks pushed as well. marking as obsolete to prevent it showing up in pending patch queries.
I've written a new patch cleaning remaining calls to Sound::Beep and unused include. I've also removed sound.hxx and sound.cxx as they were no more used. I've built master with this patch without warnings on the modified files but there is still no logging added. There is still calls to GetDisplay()->Beep() and gdk_display_beep(getGdkDisplay()), they are not directly related to fdo#48549 but they maybe deserve to be removed too?
Created attachment 64931 [details] Patch for fdo#48549 System::Beep() removal v3
Nice work! Pushed it. The remaining calls you noticed could be removed, as well as the corresponding implementations, meaning: WinSalFrame::Beep GtkSalFrame::Beep X11SalFrame::Beep SalDisplay::Beep IosSalFrame::Beep SvpSalFrame::Beep and all the virtual declarations. Seems that vcl/sndstyle.hxx could go after that cleanup too.
Thomas Arnhold committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/binfilter/commit/?id=b4f09b1d25687bc648231f9fd52528d7fd6a053c fdo#48549 Remove Sound::Beep
As Thomas Arnhold suggested, I've removed the following methods: WinSalFrame::Beep GtkSalFrame::Beep X11SalFrame::Beep SalDisplay::Beep IosSalFrame::Beep SvpSalFrame::Beep I've also removed sndstyle.hxx and calls to MessageBeep(). I've done a complete build without warnings on modified files. Thomas, if you know some similar tasks let me know.
Created attachment 64966 [details] patch removing unnecessary beep related methods
Mathieu Vonlanthen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=85cb9084533605657aca0394afe4516058a8e4ef fdo#48549 System::Beep() removal
Comment on attachment 64966 [details] patch removing unnecessary beep related methods Awesome! I let you know :)
I asked for a review concerning ACCESSIBILITY impact on <accessibility@global.libreoffice.org>
Not an issue for ACCESSIBLITY, where for Assistive Technology (AT) we should not be directly generating audible queues for input or alert events. There are specific XAccessible roles for that purpose in UNO AAPI that is the correct way to interact with AT. Other sounds by LibreOffice are disruptive.
Migrating Whiteboard tags to Keywords: (easyhack) [NinjaEdit]