Re: RFR: 8271054: [REDO] Wrong stage gets focused after modal stage creation [v9]

[Thiago Milczarek Sayao]

On Tue, 8 Mar 2022 13:12:03 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> Thiago Milczarek Sayao has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Capture event serial on process_key
>
> 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.

GDK_CURRENT_TIME is always  0, but it looks better. Changed it.

> 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;

You're right, it can avoid other possible errors.

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

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