Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 367923

Summary:	Adjust RAP JFace code to use CANCEL_KEYS instead of doit=false for KeyEvents
Product:	[RT] RAP	Reporter:	Tim Buschtoens <tbuschto>
Component:	RWT	Assignee:	Project Inbox <rap-inbox>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	tbuschto
Version:	1.5
Target Milestone:	1.5 M5
Hardware:	All
OS:	All
Whiteboard:
Bug Depends on:	367869
Bug Blocks:	330461

Description Tim Buschtoens

2012-01-05 04:41:23 EST

Support for doit=false in KeyEvents will likely be dropped (Bug 367871) and replaced with CANCEL_KEYS (Bug 367869). We should adjust our JFace code accordingly.

Comment 1 Ivan Furnadjiev

2012-01-06 04:41:48 EST

I started the evaluation of JFace/Workbench usage of KeyEvent doit = false. I will put some notes (document the classes involved) before the real re-factoring take place:
 - KeySequenceText#KeyTrapListener#handleEvent cancel all key events after handle them manually. The key handling logic is very complicated there. As this class is *not* in use by JFace/Workbench code we could keep this code untouched.
 - KeySequenceText#TraversalFilter#handleEvent - cancel TRAVERSE_RETURN and all TRAVERSE_TAB_NEXT/TRAVERSE_TAB_PREVIOUS with modifier different than SHIFT.
 - ContentProposalAdapter#TargetControlListener - cancel ESC, CR and TAB. The code that cancels other keys is currently commented. 
 - ContentProposalAdapter#addControlListener - cancel triggerKeyStroke defined by the user. As we have plans to completely rework the ContentProposalAdapter functionality (see bug 330461) we could defer it.

Comment 2 Ivan Furnadjiev

2012-01-06 05:02:55 EST

- ColumnViewerEditor#processTraverseEvent - cancel TRAVERSE_TAB_PREVIOUS and TRAVERSE_TAB_NEXT
- ComboBoxCellEditor#createControl - cancel TRAVERSE_ESCAPE and TRAVERSE_RETURN
- ComboBoxViewerCellEditor#createControl - cancel TRAVERSE_ESCAPE and TRAVERSE_RETURN
- TextCellEditor#createControl - cancel TRAVERSE_ESCAPE and TRAVERSE_RETURN
- SWTFocusCellManager#handleKeyDown - cancel keys defined in CellNavigationStrategy#shouldCancelEvent (default are ARROW_LEFT and ARROW_RIGHT, but can be changed with a custom navigation strategy)

Comment 3 Ivan Furnadjiev

2012-01-06 06:04:49 EST

- FilteredPreferenceDialog#activeKeyScrolling - cancel everything with display filter if "Key Scrolling" action is activated.
- WorkbenchKeyboard#processKeyEvent - cancel all *consumed* key binding events. The logic to cancel or not the event is in WorkbenchKeyboard#press and it's very complicated. Currently we cancel all defined key binding events (BindingManager#updateKeyBindingList).

Comment 4 Ivan Furnadjiev

2012-01-06 06:17:54 EST

Forms:
- ExpandableComposite - cancel ARROW_UP and ARROW_DOWN
- FormText - cancel TRAVERSE_TAB_NEXT and TRAVERSE_TAB_PREVIOUS in a condition.

Comment 5 Ivan Furnadjiev

2012-01-06 06:39:23 EST

Currently, I can see two potential problems:
1. Possibility to cancel *all* key events on a specific control.
2. Cancel a key event in a condition.

Comment 6 Tim Buschtoens

2012-01-09 04:41:18 EST

1. So how about a whitelist as a solution? Or allow a wildcard in CANCEL_KEYS?
2. Is this critical? Could we solve it with a piece of client-side code?

Comment 7 Ivan Furnadjiev

2012-01-12 12:02:11 EST

JFace code has been adopted to use CANCEL_KEYS instead of doit = false in key/traverse listeners. The following classes stay unchanged:
KeySequenceText
FilteredPreferenceDialog
WorkbenchKeyboard
FormText
Changes are in CVS HEAD.

Comment 8 Ivan Furnadjiev

2012-01-25 08:37:00 EST

I will close it as fixed. JFace code has been adapted, where possible.