On Tue, 25 Jan 2022 23:57:09 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org> 
wrote:

>> Found the problem thru this path:
>> 
>> **WindowStage.java**
>> 
>>     final void handleFocusDisabled() {
>>         if (activeWindows.isEmpty()) {
>>             return;
>>         }
>>         WindowStage window = activeWindows.get(activeWindows.size() - 1);
>>         window.setIconified(false);
>>         window.requestToFront();
>>         window.requestFocus();
>>     }
>> 
>> 
>> **glass_window.cpp**
>> 
>> void WindowContextBase::process_focus(GdkEventFocus* event) {
>>    ...
>> 
>>     if (jwindow) {
>>         if (!event->in || isEnabled()) {
>>             mainEnv->CallVoidMethod(jwindow, jWindowNotifyFocus,
>>                     event->in ? 
>> com_sun_glass_events_WindowEvent_FOCUS_GAINED : 
>> com_sun_glass_events_WindowEvent_FOCUS_LOST);
>>             CHECK_JNI_EXCEPTION(mainEnv)
>>         } else {
>>             mainEnv->CallVoidMethod(jwindow, jWindowNotifyFocusDisabled);
>>             CHECK_JNI_EXCEPTION(mainEnv)
>>         }
>>     }
>> }
>> 
>> 
>> So `glass_window.cpp` was triggering `jWindowNotifyFocusDisabled` which 
>> triggered the java code on the Primary Stage (on the bug reproduce code).
>> 
>> The docs states:
>> 
>>     /**
>>      * Enables or disables the window.
>>      *
>>      * A disabled window is unfocusable by definition.
>>      * Also, key or mouse events aren't generated for disabled windows.
>>      *
>>      * When a user tries to activate a disabled window, or the window gets
>>      * accidentally brought to the top of the stacking order, the window
>>      * generates the FOCUS_DISABLED window event. A Glass client should react
>>      * to this event and bring the currently active modal blocker of the
>>      * disabled window to top by calling blocker's minimize(false), 
>> toFront(),
>>      * and requestFocus() methods. It may also 'blink' the blocker window to
>>      * further attract user's attention.
>>      *
>>      .....
>> 
>> 
>> So I guess the C++ side looks ok, java side on `handleFocusDisabled` I did 
>> not understand why it `requestToFront` and `requestFocus` on the latest 
>> window.
>> 
>> The solution makes disabled windows unfocusable and prevents mouse and 
>> keyboard events as the docs states, making it compatible with other 
>> platforms.
>
> Thiago Milczarek Sayao has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Capture event serial on process_key

The fix itself looks good. though I have a query. I did not observe any 
mis-behaviour, so please check if the suggested change is required or not.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 780:

> 778:     }
> 779: 
> 780:     event_serial = 0;

should we use GDK_CURRENT_TIME instead of 0 ?
GDK_CURRENT_TIME is defined also as 0 and represent current time.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1390:

> 1388:         // prevents activeWindows on WindowStage.java to be out of 
> order which may cause the FOCUS_DISABLED event
> 1389:         // to bring up the wrong window (it brings up the last which 
> will not be the real "last" if out of order).
> 1390:         gtk_window_present_with_time(GTK_WINDOW(gtk_widget), 
> event_serial);

`event_serial` is not reset after use. I could observe that a stale value of 
`event_serial` gets reused several times.
Steps to observe the behavior:
1. Print event_serial in this method
2. Run the sample program attached to JBS.
3. Press the "Click Me!" button 
4. Answer YES on the Alert 
5. Three stages should be visible on the screen.
6. Relocate such that all stages are visible
7. Click on 'This should be on Top' Stage
8. Click on "This is a stage with no owner' stage.
9. Click on StageTest icon in Taskbar. Keep repeating this step. The stage 
gains and looses focus and the same event_serial gets printed on terminal.
=> It does not look harmful behavior wise. But can observe that `event_serial` 
gets reused.
Screenshot: 
<img width="739" alt="Screenshot 2022-03-08 at 6 53 46 PM" 
src="https://user-images.githubusercontent.com/11330676/157248115-61240f6c-1710-461e-ba60-08e6210774e7.png";>



Will it be good idea to reset it to 0/GDK_CURRENT_TIME and use something like,


if (event_serial == GDK_CURRENT_TIME) {
    gtk_window_present(GTK_WINDOW(gtk_widget));
} else {
    gtk_window_present_with_time(GTK_WINDOW(gtk_widget), event_serial);
    event_serial = GDK_CURRENT_TIME;
}


OR


gtk_window_present_with_time(GTK_WINDOW(gtk_widget), event_serial);
event_serial = GDK_CURRENT_TIME;

-------------

PR: https://git.openjdk.java.net/jfx/pull/598

Reply via email to