Bug 489541 - BindingManager removes ACTIVE_KEY/CANCEL_KEY set by the Application
Summary: BindingManager removes ACTIVE_KEY/CANCEL_KEY set by the Application
Status: NEW
Alias: None
Product: RAP
Classification: RT
Component: JFace (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-14 08:43 EDT by Michael Fröschen CLA
Modified: 2021-01-20 05:33 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Fröschen CLA 2016-03-14 08:43:20 EDT
If one defines a CANCEL KEY (as described in https://www.eclipse.org/rap/developers-guide/devguide.php?topic=key-and-mouse-events.html&version=3.0) in a Workbench, the JFace BindingManager may override this CANCEL_KEY if it has to recompute the Bindings.

A reliable way to reproduce this is by extending the Workbench Demo:

1. Add a CANCEL_KEY and/or ACTIVE_KEY to the Display. I did this in the postWindowOpen Method of the DemoWorkbenchWindowAdvisor via the following code:

    shell.getDisplay().setData( RWT.CANCEL_KEYS, new String[] {"CTRL+O"} );
    shell.getDisplay().setData( RWT.ACTIVE_KEYS, new String[] {"CTRL+O"} );
    shell.getDisplay().addFilter( SWT.KeyDown, new Listener() {
      @Override
      public void handleEvent( Event pEvent ) {
        System.out.println( "DemoWorkbenchWindowAdvisor.postWindowOpen()" );
      }
    } );

2. Run the Demo and hit CTRL+O. The Listener will be invoked correctly. (The Open File Dialog of the Browser will not be shown and instead the text is printed to the console)
3. Now open the About Dialog of the Demo and hit CTRL+O again. The Listener will not be called and instead the Browsers Open Dialog will be shown. The prevention of the Open Dialog is now gone till the page is reloaded.

This happens because the Context changes and thus the KeyBindings are recomputed. At the end of that, the BindingManager replaces the existing active and cancel keys with the keys needed for the KeyBinding, which effectively removes the active and cancel keys that have been set by the Application. This happens in the updateKeyBindingList Method of the BindingManager.

This basically forces you to create a (Dummy) KeyBinding if you want to prevent a default behavior of the browser.

I think that instead of replacing the active and cancel keys there should be a way to easily modify them (see Bug 368480) and that the BindingManager should not override existing keys.
One way would be to remove the not needed keys and then add the new keys or to use a different Data Key so that neither the Application nor the BindingManager may override each other.
Comment 1 Ivan Furnadjiev CLA 2016-03-14 09:08:34 EDT
Setting RWT.CANCEL_KEYS/RWT.ACTIVE_KEYS directly is not recomended for a Workbench based application. You should use "org.eclipse.ui.bindings" extension point instead.
Comment 2 Michael Fröschen CLA 2016-03-14 09:37:23 EDT
So in order to prevent a Browser function i should create a Key Binding?
I don't like that approach since it feels like a Workaround and not like a real solution.

If i really want to create a Key Binding, i should actually use the extension you mentioned, but not for preventing the Browser to do something like showing the file dialog.
Comment 3 Ivan Furnadjiev CLA 2016-03-14 10:18:05 EDT
Preventing browser function with CANCEL_KEYS depends on browser. You can't prevent some native browser functions (key combinations) in one browser, other in others browsers. 
JFace key bindings depend on the current scheme. When you change the scheme, you don't know which CANCEL_KEYS/ACTIVE_KEYS bolongs the the old scheme and which are added manually directly to the display in order to preserve them.
Comment 4 Michael Fröschen CLA 2016-03-14 10:46:37 EDT
(In reply to Ivan Furnadjiev from comment #3)

That the behavior differs between different Browsers is not new (e.g. my example does not work in IE 11, neither does the KeyBinding CTRL+S for Save). This also applies to KeyBindings and thus is no argument why using CANCEL_KEY or ACTIVE_KEY should not be supported in the Workbench. Users might still want to prevent specific functions in the Browser supporting this.

Regarding the change of the scheme:
That is the reason why i proposed a different Data Key.
Unfortunately that approach feels like it would be repetitive and would add additional pageload. But i can't think of another solution to distinguish the KeyBindings from Keys set by the Application.
Alternatively a Listener could be invoked after changing the keys so that the Application may check if it has to add keys.

In my opinion the current behavior is not intuitive and limits the usability.

And even if the usage of RWT.CANCEL_KEYS/RWT.ACTIVE_KEYS is not recommended in the Workbench, it should be described somewhere, at least in the JavaDoc.
Comment 5 Michael Heiss CLA 2021-01-20 05:33:20 EST
Mentioning that in the documentation would saved me some hours debugging why my bindings where not working anymore after opening / closing a new shell.

As a workaround i am now attaching a SWT.Activate filter to the display to overwrite whatever the BindingManager has set as active keys. This is working fine since my listener is always called after BindingManager has done its work.

It would be nice if we could get a system property to prevent the BindingManager from overwriting custom settings. We handle key-events in our application by using our own global listeners. We do not have a single binding defined in the 'ui.bindings' extension. This is working fine in case of RCP and with the workaround it is now also working fine in RAP :)