FWD » Core Development » User Interface

The issues to solve here:
1. Investigate and fix how the reaction on ENTER key implemented in 4GL.
2. Find out the alternative way to have key press/release event for SPACEBAR key. The problem is currently we have only key typed event for this key. The Java backend probably even does not emit press/release events for this case.
3. Investigate the button highlight behavior for modern Windows themes (8 and 10) and properly implement in FWD.

Also make sure the following visual behavior works correctly in FWD. When mouse is pressed over a button and mouse is hovered outside the button, the button should change to mouse-over highlighted state. In FWD, all highlights are removed from the button in this case. Also this behavior happens only in Windows 8 and 10 themes.

Hynek Cihlar wrote:

Also make sure the following visual behavior works correctly in FWD. When mouse is pressed over a button and mouse is hovered outside the button, the button should change to mouse-over highlighted state. In FWD, all highlights are removed from the button in this case. Also this behavior happens only in Windows 8 and 10 themes.

Please provide an example of what you mean. Is it about dotted rectangle inside button? Something else other?

Good plan.

- The bug was fixed following all the steps in the solving strategy. The <<on choose of button>> works as expected for space-bar pressing. However, there is still a problem avoiding the triggering of the event at the space-bar key release which duplicates the <<on " " of button>> action execution.

- To sum the things up, in case the action is defined "on space of a button", than there will be at least 2 space-bar related events (one typed and one release), causing at least 2 action executions.

- What have been already tried:

- Consuming the space-bar key released event at this stage to avoid execution. (doesn't seem to have an impact)

- Directly emitting a choose event instead of a space-bar key release  (doesn't work as it is not a real key, and the event is eliminated afterwards, before reaching the triggering)

- Tried to alter the event's data so it doesn't reassemble a space-bar event at all. (doesn't seem to change the behavior, moreover it seems the action is pushed into a queue beforehand)

- Tried to avoid using the ActionEvent at all, but no remarkable results.

Adrian, please post your changes here.

Actually, please create a task branch 3690a and put the changes there.

Branch 3690a created.
Changes committed to branch.

There is another related issue. In OpenEdge when both enter and chose triggers for a button are defined, pressing Enter key only triggers enter, while in FWD we trigger both, enter and chose.

Adrian, do the changes in 3690a revision 11321 represent your fixes for the code review items? If so, is it ready for review again?

Yes, revision 11321 of 3690a fixes the points mentioned and it is ready for review. Last reviewed revision was 11320.

Code review 3690a revisions 11321 and 11322. I'm good with the changes. Just please remove the commented out code in 11322 (unless it's required to stay in for clarity).

Indeed, revision 11321 was meant to fully fix bug #3690, while revision 11322 is still work in progress. There should be done more tests to ensure well functionality of the screen value of the combo box. (#3889)

I think it makes sense to split these changes. Please merge the 11320 and 11321 revisions into 4045a.

Then create a new 3889a branch and move revision 11322 into there. Branch 3690a can be archived as dead at that point.

Adrian Lungu wrote:

Moved revision 11322 onto a new branch 3889a. Revisions 11320 and 11321 were merged into 4045a. Branch 3690a is no longer needed at this stage.

The rev 11370 branch 4045a has a regression that the dialog message buttons are not sensitive to mouse clicking. Please test simple alert message box.

I have fixed the lack of response from the buttons inside the alert box. This was due to the fact that no ActionEvent was created on a choose key input event. This kind of event is also generated by the mouse click, which explains why there was no functionality attached to the button.

I have committed the fix in revision 11372 of branch 4045a.

There are failed CHUI regression tests with 4045a related to this issue. Pressing Space key is not properly handled. Please run this testcase and press SPACE key

MESSAGE "Press Space or Enter or Click Me." VIEW-AS ALERT-BOX.

Sergey Ivanovskiy wrote:

There are failed CHUI regression tests with 4045a related to this issue. Pressing Space key is not properly handled. Please run this testcase and press SPACE key
[...]

It can be reproduced with chui_native client.

Sergey: Can you please prepare the fix for this?

Key released events are not detected by Linux terminal using ncurses and please correct if it is not true. Thus the idea to use key released for this case should be reworked. I just looked at this article https://blog.robertelder.org/detect-keyup-event-linux-terminal/ and googled this technical issue.

I don't object to the proposed change. I do have a question. Why is it happening now? Is this the first time we relied upon the key released event for processing?

Adrian: Please review.

Greg Shah wrote:

I don't object to the proposed change. I do have a question. Why is it happening now? Is this the first time we relied upon the key released event for processing?

Yes, the key released event is used to solve #3690-7, isn't it?

Indeed, the key released event was designed especially for this. The changes look good to me as it conducts a valid behavior for the ChUI separately. I didn't had the chance to test enough the changes of #3690 on the ChUI, but the diff seem to fix the problem.

Key released events are not detected by Linux terminal using ncurses and please correct if it is not true. Thus the idea to use key released for this case should be reworked. I just looked at this article https://blog.robertelder.org/detect-keyup-event-linux-terminal/ and googled this technical issue.

I wasn't aware of such aspect at the moment of implementation, as the tests were only done on the GUI. If this however stands, it shouldn't be a problem as long as the KEY_RELEASE event is actively used only in ButtonGuiImpl.handleBasicEvents and KeyInput.canProduceAction. While ButtonGuiImpl is not used in ChUI and KeyInput is aware of the ChUI (due to the diff in #3690-28), any related issue shouldn't appear anymore.

Sergey: Please commit the change to 4045a. It fixes a regression so it is OK to apply to that frozen branch.

Committed revision 11388 (4045a). I will start a new round of the chui regression tests for rev 11388 (4045a) if there are no objections.

Sergey Ivanovskiy wrote:

I will start a new round of the chui regression tests for rev 11388 (4045a) if there are no objections.

Yes, please do.

Adrian Lungu wrote:

Current behavior:
- If an action is defined "on choose of button" and in the Swing GUI interface we hold down space focusing that button, the action will incorrectly be fired multiple times, instead of only once at the release time. However if the action is defined "on space of button", the action should be fired multiple times.
- There was no Key Release event generated for any key, fact which is crucial for solving this bug

Solving strategy:
- Create a new kind of event, namely KeyRealese.
- To avoid possible new bugs and for performance reasons, restrict the generation of such events only for space-bar keystrokes.
- In the event queue, keep only the key released events related to a ButtonGuiImpl.
- To reproduce the exact behavior of Progress, generate only one "choose event" only when the space-bar is released. (not for every space-bar typed event)
- Avoid triggering action for key released events.
- At a space-bar press and release action, there should be 2 generated user events (space-bar key typed and space-bar key released) and one artificial event (choose pressed), in this order. As a matter of fact, if space-bar will be hold for a little longer and released, there should be multiple space-bar key typed events generated, but still only one space-bar key released event and only one choose pressed event in the end.

Adrian, could you provide references for 4GL tests that proved this behaviour? I need them in order to check them with the PAUSE fixes.

Are there tests for READKEY.? Are there evidences that 4GL uses key released events for its run-time implementation?

For an example, if we press SPACE for a while, then depending on the OS drivers, the SPACE typed events will be generated as repeated events.

Committed revision 11391 (4045a) should fix #3690-36.

Sergey: Please explain the idea behind this change. The safety and correctness of the change is not obvious to me.

Adrian: Please respond to the questions above. This regression needs to be resolved with priority.

Greg Shah wrote:

Sergey: Please explain the idea behind this change. The safety and correctness of the change is not obvious to me.

Please look at TypeAHead.this.dequeueEvent() and EventManager.eventFromKey. It follows that TypeAHead.getKeystroke(int msec, boolean honorServerEvents) returns null for key released events when we press and release SPACE key, but
the run-time implementation of ThinClient.waitForEvents depends on key events. Thus these changes fixed this run-time implementation of ThinClient.waitForEvents for the case when SPACE key is pressed and then released.
Committed revision 11392 (4045a) fixed the same issue for READKEY. That can be observed with this simple test

MESSAGE "step 1".

PAUSE.

MESSAGE "step 2".

PAUSE.

MESSAGE "step 3".

READKEY.

MESSAGE "step 4".

READKEY.

MESSAGE "done".

Firstly, related to #3690-37:

Adrian, could you provide references for 4GL tests that proved this behavior? I need them in order to check them with the PAUSE fixes.

I will attach here an archive with some tests I used. Regarding specifically to the question, choose.p and space.p are the simple tests which reflect the behavior of holding the space-bar down with triggers on choose/space. However I see that #3690-42 is a fix for the issue mentioned, but the archive may still prove of good use for quick testing.

Are there tests for READKEY.? Are there evidences that 4GL uses key released events for its run-time implementation?

For an example, if we press SPACE for a while, then depending on the OS drivers, the SPACE typed events will be generated as repeated events.

I am not aware of any testcase including READKEY. Now, it is not clear if the run-time implementation of 4GL does explicitly use the KEY_RELEASE event or it is just an OS behavior. However, Linux drivers does send multiple events of KEY_TYPED/KEY_PRESSED if the key is hold.

It looked like the KEY_RELEASE event was the most natural way to solve the bug. Despite the huge implication of Key Events in the application, I retained the generation of KEY_RELEASE events just for space-bars and triggers are not fired for such events. Basically for now, these are dummies used for generating choose events or repaint. At this stage, any other use of KEY_RELEASE should be either ignored or converted into a more convenient event.

Adrian, thank you for these tests. If we hold some key down on the keyboard, then auto repeats are generated for this pressed key. This case should be taken into account when we consider 4GL behaviour.

Sergey Ivanovskiy wrote:

Adrian, thank you for these tests. If we hold some key down on the keyboard, then auto repeats are generated for this pressed key. This case should be taken into account when we consider 4GL behaviour.

Are there additional deviations between FWD and the 4GL?

Please look at TypeAHead.this.dequeueEvent() and EventManager.eventFromKey. It follows that TypeAHead.getKeystroke(int msec, boolean honorServerEvents) returns null for key released events when we press and release SPACE key, but the run-time implementation of ThinClient.waitForEvents depends on key events.

I've reviewed this code as well as EventManager.getKeyEvent() and KeyInput. It is not clear to me that a key released event will be returned as null. Can you explain it further?

My other concern: if we ever return null from TypeAhead in other cases besides KEY_RELEASED, then this new code may cause unexpected behavior. It is not obvious that null is only ever a KEY_RELEASED case.

Greg Shah wrote:

Sergey Ivanovskiy wrote:

Adrian, thank you for these tests. If we hold some key down on the keyboard, then auto repeats are generated for this pressed key. This case should be taken into account when we consider 4GL behaviour.

Are there additional deviations between FWD and the 4GL?

I don't understand now why the action for SPACE is triggered on release key event. According to these tests if we press SPACE on the button (ref. space.p), then the corresponding action is triggered one time.

Greg Shah wrote:

Please look at TypeAHead.this.dequeueEvent() and EventManager.eventFromKey. It follows that TypeAHead.getKeystroke(int msec, boolean honorServerEvents) returns null for key released events when we press and release SPACE key, but the run-time implementation of ThinClient.waitForEvents depends on key events.

I've reviewed this code as well as EventManager.getKeyEvent() and KeyInput. It is not clear to me that a key released event will be returned as null. Can you explain it further?

My other concern: if we ever return null from TypeAhead in other cases besides KEY_RELEASED, then this new code may cause unexpected behavior. It is not obvious that null is only ever a KEY_RELEASED case.

Adrian, please explain this code.

Sergey Ivanovskiy wrote:

Greg Shah wrote:

Please look at TypeAHead.this.dequeueEvent() and EventManager.eventFromKey. It follows that TypeAHead.getKeystroke(int msec, boolean honorServerEvents) returns null for key released events when we press and release SPACE key, but the run-time implementation of ThinClient.waitForEvents depends on key events.

I've reviewed this code as well as EventManager.getKeyEvent() and KeyInput. It is not clear to me that a key released event will be returned as null. Can you explain it further?

My other concern: if we ever return null from TypeAhead in other cases besides KEY_RELEASED, then this new code may cause unexpected behavior. It is not obvious that null is only ever a KEY_RELEASED case.

Adrian, please explain this code.

Sorry, yes, the null value for READKEY and PAUSE statement seems can be from key released events. I don't know.

OK. I understand now that if we press SPACE, then the triggered action is executed at the moment when this key is released. (ref: space.p) That is why the key released is used now.

Greg Shah wrote:

Please look at TypeAHead.this.dequeueEvent() and EventManager.eventFromKey. It follows that TypeAHead.getKeystroke(int msec, boolean honorServerEvents) returns null for key released events when we press and release SPACE key, but the run-time implementation of ThinClient.waitForEvents depends on key events.

I've reviewed this code as well as EventManager.getKeyEvent() and KeyInput. It is not clear to me that a key released event will be returned as null. Can you explain it further?

My other concern: if we ever return null from TypeAhead in other cases besides KEY_RELEASED, then this new code may cause unexpected behavior. It is not obvious that null is only ever a KEY_RELEASED case.

I found another solution that fixed EventManager.eventFromKey for released keys events. Please review the committed rev. 11393. It should overcome this case that you pointed.

Yes, this is much better. The intention is clear.

Please do the following:

• run ChUI regression testing
• check the standalone testcases related to this issue
• check the big GUI app smoke tests

Greg Shah wrote:

• run ChUI regression testing

CHUI main regression tests passed for rev 11393 (4045a) in one batch run and two single runs.

• check the standalone testcases related to this issue

Seems to be OK (passed).

• check the big GUI app smoke tests

The calculated values for the smoke test 1 differs from the one we have on the video. I can't check that all calculated values are OK in these tests.

I didn't think about LASTKEY functionality. Does it work properly? Released keys events can affect this value even if the CHUI tests were passed.

Please make changes to exclude LASTKEY from effects by key release.

Greg Shah wrote:

Please make changes to exclude LASTKEY from effects by key release.

I am looking into lastevent/ for test cases and lastkey_reset.p seems to be important.

I recently found a bug related to the issue here - regarding the web interface. The web driver doesn't emit release events, which means the widgets which use the released keys, won't work properly. By now, the button is the only widget which is sensible to those (that is why typing space on the button will only paint it without firing the action).

At first glance, a change similar to the one done in the swing driver should be adopted (generate release key events for space-bar). However, the web driver doesn't seem to be that flexible, letting the release events propagate to the server. In fact, it looks like a more complex system is built around the onkeyup and onkeydown events.

I logged the type of event which arrives at sendKey(key, evt) - the high end method for sending key events to the server in p2j.keyboard.js.
For a simple space-bar type on a button:

keyup

keydown
keydown
keyup
keydown
keydown
keydown
keyup
keydown
keyup
keydown
keydown
keydown
keydown
keydown
keyup

1. send 2 events when typing a key (press and release / keyup and keydown)
2. understand why "random" events are sent to the server when holding down key (both keyup and keydown)

Even though the KEY_RELEASED event is not a progress event, it should be generated in order to properly reproduce the default behaviors of Windows widgets. Also, I am not sure now if this is the only case in which this is required - there may be also other widgets with Windows implementation which makes use of released keys.

Ultimately, I can't say this is a high priority issue yet - but it can be a reference point when dealing with KEY_RELEASED dependent widgets.

Whatever solution is created, please minimize the number of events forwarded to the FWD client (Java side). Optimally, we would be able to go into one location in the javascript code to enable key release support for a particular key.

Adrian Lungu wrote:

I recently found a bug related to the issue here - regarding the web interface. The web driver doesn't emit release events, which means the widgets which use the released keys, won't work properly. By now, the button is the only widget which is sensible to those (that is why typing space on the button will only paint it without firing the action).

At first glance, a change similar to the one done in the swing driver should be adopted (generate release key events for space-bar). However, the web driver doesn't seem to be that flexible, letting the release events propagate to the server. In fact, it looks like a more complex system is built around the onkeyup and onkeydown events.

I logged the type of event which arrives at sendKey(key, evt) - the high end method for sending key events to the server in p2j.keyboard.js.
For a simple space-bar type on a button:
[...]
For a single longer press and release of the space-bar on a button - which seems strange:
[...]

Yet, I am not sure how the key events are handled in the web driver. But from this slim investigation, it seems that major changes should be done in order to:

1. send 2 events when typing a key (press and release / keyup and keydown)
2. understand why "random" events are sent to the server when holding down key (both keyup and keydown)

Even though the KEY_RELEASED event is not a progress event, it should be generated in order to properly reproduce the default behaviors of Windows widgets. Also, I am not sure now if this is the only case in which this is required - there may be also other widgets with Windows implementation which makes use of released keys.

Ultimately, I can't say this is a high priority issue yet - but it can be a reference point when dealing with KEY_RELEASED dependent widgets.

It seems that the repeats keys are generated when you press and hold the key.

It seems that keyup browser's event means that a key is released. Is it correct to change this part of the js web client part? This part has special program to test keystokes testcases/uast/keyboards/web.

Adrian Lungu wrote:

Yet, I am not sure how the key events are handled in the web driver. But from this slim investigation, it seems that major changes should be done in order to:

1. send 2 events when typing a key (press and release / keyup and keydown)
2. understand why "random" events are sent to the server when holding down key (both keyup and keydown)

It is not random. Please provide what is the test case that proves that the events are generated incorrectly.

Even though the KEY_RELEASED event is not a progress event, it should be generated in order to properly reproduce the default behaviors of Windows widgets. Also, I am not sure now if this is the only case in which this is required - there may be also other widgets with Windows implementation which makes use of released keys.

Please handle this on the java web client side.

Even though the KEY_RELEASED event is not a progress event, it should be generated in order to properly reproduce the default behaviors of Windows widgets. Also, I am not sure now if this is the only case in which this is required - there may be also other widgets with Windows implementation which makes use of released keys.

Please handle this on the java web client side.

I don't want to send a large number of additional events up to Java if we only need a very small number of cases. For example, if we only need to send keyUp for space, then we would not send keyUp events for all keys. Otherwise we are creating a new performance issue.

Now I see that this behavior can't be reproduced on Chrome. Maybe it is a bug in Firefox regarding long key presses?

After checking the GUIKeyboardReader, I see there is the following code which triggers that additional keyups:

if (evt.keyCode === keys.IME_PROCESSKEY)
{
    var keyUpEvt = p2j.createKeyEvent("keyup", false, true, null, evt.ctrlKey,
       evt.altKey, evt.shiftKey, evt.metaKey, repeatCode, 0);

    // check if the keyboard event is created successfully
    if (keyUpEvt)
    {
        onkeyup(keyUpEvt);
    }

    p2j.consumeEvent(evt);
    return;
}

I couldn't find very strong documentation regarding keys.IME_PROCESSKEY (229 key code), but I guess this was the reason why Firefox and Chrome showed different behaviors. (Firefox fires these events - for me right now seem random, but maybe they are not). With this observation, this means the space-bar alone is working properly: sending continuous keydown signals and one keyup signal at the end.

Committed fix in 4231b/rev. 11373. The changes were minimal - only the space-bar generates additional released events.

Code Review Task Branch 4231b Revision 11372

The changes seem good to me.

Sergey: Any objections?

It seems the changes in 4231b Revision 11372 are good.

This is a simple test page for keyboard reader for branch 3821c. Please place them in 3821c project root with these two js: keystrokes.js and keyboard_testcases.js and open this html page in the browser. There are several automatic tests and manual gui test, click on the "GUI Manual Testing" button and click on the input field, then press any combination and look at the Progress 4GL generated code. In this branch pressing SPACE produces two keycodes 32. It is a real regression and it isn't present in the old versions.

I didn't observe that it was due to the changes related SPACE processing. The browser generates SPACE key down, SPACE key pressed and SPACE key released. Does it make sense to send SPACE key DOWN when it was down and to send SPACE key RELEASED when it was released?
Finally, ALT+SPACE produces two events. Is it correct?

Sergey: I think we should have this test app built into FWD and expose it for testing purposes. Do we want to expose it on the FWD server or client? The client might be best because we could access it via menu item on virtual desktop and it would never conflict with the server-side customer web apps.

Sergey Ivanovskiy wrote:

I didn't observe that it was due to the changes related SPACE processing. The browser generates SPACE key down, SPACE key pressed and SPACE key released. Does it make sense to send SPACE key DOWN when it was down and to send SPACE key RELEASED when it was released?
Finally, ALT+SPACE produces two events. Is it correct?

The provided application proves to be very useful for this kind of matters. The KEY_RELEASED event is generated only by SPACE and used only by the button widget. Other widgets will instantly consume the event and ignore it. Other keys do not generate KEY_RELEASED event.

Related to the DOWN and RELEASED events, these are needed for esthetics at first. The button should be drawn as pressed when DOWN is processed. The button should be redrawn when released and eventually fire a trigger. Also, note that holding the SPACE key will generate several 32 key codes (mostly DOWN events).

Related to ALT+SPACE, I guess the driver should avoid generating release events when the SPACE is part of a key code with modifiers. I can't identify a testcase to reproduce a bug exploiting this. I will fix double events for ALT+SPACE at the driver level.

Adrian Lungu wrote:

Sergey Ivanovskiy wrote:

I didn't observe that it was due to the changes related SPACE processing. The browser generates SPACE key down, SPACE key pressed and SPACE key released. Does it make sense to send SPACE key DOWN when it was down and to send SPACE key RELEASED when it was released?
Finally, ALT+SPACE produces two events. Is it correct?

The provided application proves to be very useful for this kind of matters. The KEY_RELEASED event is generated only by SPACE and used only by the button widget. Other widgets will instantly consume the event and ignore it. Other keys do not generate KEY_RELEASED event.

Related to the DOWN and RELEASED events, these are needed for esthetics at first. The button should be drawn as pressed when DOWN is processed. The button should be redrawn when released and eventually fire a trigger. Also, note that holding the SPACE key will generate several 32 key codes (mostly DOWN events).

Related to ALT+SPACE, I guess the driver should avoid generating release events when the SPACE is part of a key code with modifiers. I can't identify a testcase to reproduce a bug exploiting this. I will fix double events for ALT+SPACE at the driver level.

What is the driver level? Please use for testing the test page attached in this Redmine thread #3690-75, #3690-73.

Greg Shah wrote:

Sergey: I think we should have this test app built into FWD and expose it for testing purposes. Do we want to expose it on the FWD server or client? The client might be best because we could access it via menu item on virtual desktop and it would never conflict with the server-side customer web apps.

I used the standalone test page for testing what key codes are produced by p2j.keyboard.js. Please review the files attached in #3690-75. It seems it is not required to start the server and the web client to test what Progress 4GL keys are generated by js web client.

Removed key released events for SPACE with modifiers in 3821c/rev. 11430 as mentioned in #3690-77 to avoid ALT+SPACE double event creation.

Sergey Ivanovskiy wrote:

What is the driver level? Please use for testing the test page attached in this Redmine thread #3690-75, #3690-73.

I was referring to WinKeyboardReader and p2j.keyboard.js. The test page shows that ALT+SPACE (1056) is now generated only once.

Sergey Ivanovskiy wrote:

Greg Shah wrote:

Sergey: I think we should have this test app built into FWD and expose it for testing purposes. Do we want to expose it on the FWD server or client? The client might be best because we could access it via menu item on virtual desktop and it would never conflict with the server-side customer web apps.

I used the standalone test page for testing what key codes are produced by p2j.keyboard.js. Please review the files attached in #3690-75. It seems it is not required to start the server and the web client to test what Progress 4GL keys are generated by js web client.

Yes, I understand. My point is that I want this built into FWD so we don't have to spend time trying to recreate this application in order to test. If you want to add this to the FWD server, that is OK but I worry that the URL can conflict with customer web URLs. So perhaps you should expose it with something very unique like "fwd_javascript_keyboard_test_application".