InvokingKeyBindings event not invoked in OnInvokingKeyBindings overridden method.

[BDisp]

ToDos

• Make OnInvokingKeyBindings protected
• Make OnKeyDown/OnKeyUp protected

Background

Here a prove that the keyboard implementation still failing to invoke events before the derived class. The original unit test is asserting for the Assert.True (view.NewKeyDownEvent (Key.A));. In the TextView there is no key bindings that handles directly the Key.A and thus it return false from the OnInvokingKeyBindings overridden method and the TextView insert the 'a' char when the OnProcessKeyDown method is called, so the ProcessKeyDown is invoked. If we use the Key.Backspace the TextView will handle it on the OnInvokingKeyBindings and return true, so the InvokingKeyBindings event will never be called.
In this way is impossible to code a flow where we can catch the keys in a event before sending to the derived class and the VkeyPacketSimulator scenario will be hard to fix. When we press a command like Ctrl+Shit+Alt+A, which isn't handled by TextView, a message box will opened with the information of the keys pressed.
Try run the follow changed unit test and it will fail on TextView or create a global command that handle the Key.A and probably all views will fail.

	[Fact]
	public void AllViews_KeyDown_All_EventsFire ()
	{
		foreach (var view in TestHelpers.GetAllViews ()) {
			if (view == null) {
				_output.WriteLine ($"ERROR: null view from {nameof (TestHelpers.GetAllViews)}");
				continue;
			}
			_output.WriteLine ($"Testing {view.GetType ().Name}");

			bool keyDown = false;
			view.KeyDown += (s, a) => {
				a.Handled = false; // don't handle it so the other events are called
				keyDown = true;
			};

			bool invokingKeyBindings = false;
			view.InvokingKeyBindings += (s, a) => {
				a.Handled = false; // don't handle it so the other events are called
				invokingKeyBindings = true;
			};

			bool keyDownProcessed = false;
			view.ProcessKeyDown += (s, a) => {
				a.Handled = true;
				keyDownProcessed = true;
			};

			if (view is TextView) {
				Assert.True (view.NewKeyDownEvent (Key.Backspace));
			} else {
				Assert.True (view.NewKeyDownEvent (Key.A)); // this will be true because the ProcessKeyDown event handled it
			}
			Assert.True (keyDown);
			Assert.True (invokingKeyBindings);
			Assert.True (keyDownProcessed);
			view.Dispose ();
		}
	}

Originally posted by @BDisp in #3054 (comment)

[BDisp]

I know the pattern, although not mandatory, is invoke an event inside the virtual OnEvent method but for the event been invoked by an overridden method it have to call the base.OnEvent. Do you trust that all inherits class do this? I don't trust. I agree that only the base and the sub-class can access that method, to avoid a user that use it from fire the event themself and thus they must be protected virtual methods. If its need to prevent a event from firing on an overridden method, then the base class must invoke it inside the OnEvent, which I think it isn't the intention on our case, right?

[tig]

@BDisp please let me know if these Todos capture this issue properly:

ToDos

• Make OnInvokingKeyBindings protected
• Make OnKeyDown/OnKeyUp protected

[BDisp]

@BDisp please let me know if these Todos capture this issue properly:

ToDos

• Make OnInvokingKeyBindings protected

What else?

OnKeyDown
OnKeyUp