Issue 105676 - Pass app-wide shortcuts on, also in open dialogs
Summary: Pass app-wide shortcuts on, also in open dialogs
Alias: None
Product: gsl
Classification: Code
Component: code (show other issues)
Version: current
Hardware: Unknown All
: P3 Trivial (vote)
Target Milestone: ---
Assignee: philipp.lohmann
QA Contact: issues@gsl
Keywords: usability
Depends on:
Reported: 2009-10-07 10:49 UTC by thb
Modified: 2011-03-15 16:03 UTC (History)
7 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---

Proposed fix (901 bytes, patch)
2009-10-07 10:50 UTC, thb
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description thb 2009-10-07 10:49:26 UTC
let ctrl-<keycode> stuff pass dlg keyboard handling, to permit global
application-wide shortcuts also in open dialogs.

This was added specifically for Impress, as the animation tool pane is
internally implemented as a dialog - if that got focus, e.g. ctrl-s just
silently doesn't work (and yeah, sometimes even Impress saves quick enough that
I did _not_ notice there was no save happening at all).
Comment 1 thb 2009-10-07 10:50:10 UTC
Created attachment 65200 [details]
Proposed fix
Comment 2 thb 2009-10-07 11:20:53 UTC
@fl: consider adding this to your renaissance query, too. We're coming from
Impress here.
Comment 3 philipp.lohmann 2009-11-05 15:46:41 UTC
fl: could you please comment on this patch ? Go or no go ?
Comment 4 philipp.lohmann 2010-04-21 16:51:59 UTC
@fl: ping ? you're ruining gsl's IIT statistic.
Comment 5 philipp.lohmann 2010-04-21 17:02:34 UTC
adding mba,nn,cl on CC

the consequence of this would of course be that e.g. saving can be done during
an open dialog. This and similar actions might produce yet unknown reentrance
problems which would have to be fixed; what's the application developer's
general take on this ?
Comment 6 Mathias_Bauer 2010-04-21 17:24:35 UTC
We allow "global" application-wide shortcuts to work in non-modal GUI elements
like ModelessDialogs or FloatingWindows. If it required special code in Impress
to make that work then probably because Impress didn't use the sfx classes that
implemented that feature.

IMHO we shouldn't allow that in modal dialogs. In case a dialog is modal by
intent it can be a disaster to allow for global shortcut processing. If the
dialog is just modal because of laziness, perhaps the best fix for the problem
would be to convert the dialog into a modeless one. Once we have got rid of all
modal dialogs that don't need to stay modal, this issue will be fixed in passing.

In short words: "modal" is "modal" and processing shortcuts in application code
will be a tunnel through this modality. 
Comment 7 philipp.lohmann 2010-04-22 15:06:19 UTC
mba's argument seams reasonable to me. So that would mean this patch is not the
desired solution for the problem at hand. Agreed ?
Comment 8 clippka 2010-04-22 15:14:39 UTC
which sfx class implement that feature?

-1 for the patch. A application wide shortcut could be any combination, not only
Comment 9 Mathias_Bauer 2010-04-22 17:14:04 UTC
Here's the implementation for the class SfxDockingWindows:

long SfxDockingWindow::Notify( NotifyEvent& rEvt )
    if ( rEvt.GetType() == EVENT_GETFOCUS )
              // snip code
    else if( rEvt.GetType() == EVENT_KEYINPUT )
        if ( !DockingWindow::Notify( rEvt ) && SfxViewShell::Current() )
	return TRUE;

    // snip more code

    return DockingWindow::Notify( rEvt );

While checking this code I discovered that nowadays it handles *all*
accelerators. It seems that the behavior has been changed quite some time ago
(perhaps OOo2.0 when SFX lost control over accelerator configuration).
Comment 10 thb 2010-04-23 23:41:19 UTC
@mba: yep, good point - so e.g. sd/source/ui/animations/CustomAnimationPane.src
sets DialogControl = TRUE, and parent windows don't, to make vcl handle
focus/tab travelling in that pane, presumably.
Unfortunately that leads to the described behaviour of eaten global shortcuts -
is there an easy filter I could use in one of the parent windows' Notify method,
to weed out app shortcuts (and only those)?
Comment 11 philipp.lohmann 2010-04-26 10:18:58 UTC
removing patch flag
Comment 12 Mathias_Bauer 2010-05-17 21:04:12 UTC
As I wrote, I opt for not hacking something into modal dialogs but instead of
that converting them to modeless ones. In that case it seems that there is no
*basic* problem with allowing to handle all shortcuts.

But allowing for shortcuts while a modal dialog is executed is asking for troubles.

The problem is not the dialog itself, it's the code stack calling execute().