Re: JDK-8220272: Found the problem + possible fix

2019-04-15 Thread Kevin Rushforth

Hi Thiago,

If you are interested in contributing a fix, please see CONTRIBUTING.md 
[1] for instructions. Before we can evaluate your proposed fix, we will 
need is a signed OCA [2] from you.


Thank you.

-- Kevin

[1] 
https://github.com/javafxports/openjdk-jfx/blob/develop/.github/CONTRIBUTING.md

[2] http://www.oracle.com/technetwork/community/oca-486395.html

On 4/14/2019 5:46 PM, Thiago Milczarek Sayao wrote:

* THE PROBLEM *

In GlassApplication.cpp, the GDK_FOCUS_CHANGE event gets called from the last window to the first 
on my setup (I suspect "out-of-order" on some setups and even occasionally in the 
expected order). So the code to keep the latest enabled window fails (refering to WindowStage.java 
activeWindows) - that's why the "First Window" (on the code to reproduce the bug report) 
shows on top - because openjfx thinks it was the latest window. In WindowStage.java:

     final void handleFocusDisabled() {
     if (activeWindows.isEmpty()) {
     return;
     }
     WindowStage window = activeWindows.get(activeWindows.size() - 1); 
//<-- HERE!!
     window.setIconified(false);
     window.requestToFront();
     window.requestFocus();
     }

When there is a WINDOW_MODAL this code gets called because the modal window 
disable it's parents (so they do not get events - it's correct). So openjfx 
brings up what it thinks it's the latest window - but since the out of order 
GDK_FOCUS_CHANGE - it's the wrong window (activeWindows list has not the 
correct order).

The Gtk docs states GDK_FOCUS_CHANGE is true "if the window has gained the keyboard 
focus" which may not be directly correlated to openjfx's WindowEvent.FOCUS_GAINED.

* THE FIX *

To fix this, WindowEvent.FOCUS_GAINED must be called in order of which window 
is shown (because the window is inserted on activeWindows at this event).

So I have modified glass_window.cpp to immediately call 
WindowEvent.FOCUS_GAINED when showing the window:

void WindowContextBase::set_visible(bool visible) {
 if (visible) {
 gtk_widget_show_all(gtk_widget);

 //JDK-8220272 - fire event first because GDK_FOCUS_CHANGE is not 
always in order
 if(jwindow && isEnabled()) {
 mainEnv->CallVoidMethod(jwindow, jWindowNotifyFocus, 
com_sun_glass_events_WindowEvent_FOCUS_GAINED);
 }
 } else {
 gtk_widget_hide(gtk_widget);
 if (jview && is_mouse_entered) {
 is_mouse_entered = false;
 mainEnv->CallVoidMethod(jview, jViewNotifyMouse,
 com_sun_glass_events_MouseEvent_EXIT,
 com_sun_glass_events_MouseEvent_BUTTON_NONE,
 0, 0,
 0, 0,
 0,
 JNI_FALSE,
 JNI_FALSE);
 CHECK_JNI_EXCEPTION(mainEnv)
 }
 }
}

Did some tests and it seems to fix the problem.

I do not have permission to make a PR, so posting the fix here. Will happily do 
the PR if required.

Thanks.





Re: JDK-8220272: Found the problem + possible fix

2019-04-15 Thread Nir Lisker
Hi Thiago,

Anyone can create a PR on GitHub, see
https://github.com/javafxports/openjdk-jfx/blob/develop/.github/CONTRIBUTING.md
.

- Nir

On Mon, Apr 15, 2019 at 3:47 AM Thiago Milczarek Sayao <
thiago.sa...@clamed.com.br> wrote:

> * THE PROBLEM *
>
> In GlassApplication.cpp, the GDK_FOCUS_CHANGE event gets called from the
> last window to the first on my setup (I suspect "out-of-order" on some
> setups and even occasionally in the expected order). So the code to keep
> the latest enabled window fails (refering to WindowStage.java
> activeWindows) - that's why the "First Window" (on the code to reproduce
> the bug report) shows on top - because openjfx thinks it was the latest
> window. In WindowStage.java:
>
> final void handleFocusDisabled() {
> if (activeWindows.isEmpty()) {
> return;
> }
> WindowStage window = activeWindows.get(activeWindows.size() - 1);
> //<-- HERE!!
> window.setIconified(false);
> window.requestToFront();
> window.requestFocus();
> }
>
> When there is a WINDOW_MODAL this code gets called because the modal
> window disable it's parents (so they do not get events - it's correct). So
> openjfx brings up what it thinks it's the latest window - but since the out
> of order GDK_FOCUS_CHANGE - it's the wrong window (activeWindows list has
> not the correct order).
>
> The Gtk docs states GDK_FOCUS_CHANGE is true "if the window has gained the
> keyboard focus" which may not be directly correlated to openjfx's
> WindowEvent.FOCUS_GAINED.
>
> * THE FIX *
>
> To fix this, WindowEvent.FOCUS_GAINED must be called in order of which
> window is shown (because the window is inserted on activeWindows at this
> event).
>
> So I have modified glass_window.cpp to immediately call
> WindowEvent.FOCUS_GAINED when showing the window:
>
> void WindowContextBase::set_visible(bool visible) {
> if (visible) {
> gtk_widget_show_all(gtk_widget);
>
> //JDK-8220272 - fire event first because GDK_FOCUS_CHANGE is not
> always in order
> if(jwindow && isEnabled()) {
> mainEnv->CallVoidMethod(jwindow, jWindowNotifyFocus,
> com_sun_glass_events_WindowEvent_FOCUS_GAINED);
> }
> } else {
> gtk_widget_hide(gtk_widget);
> if (jview && is_mouse_entered) {
> is_mouse_entered = false;
> mainEnv->CallVoidMethod(jview, jViewNotifyMouse,
> com_sun_glass_events_MouseEvent_EXIT,
> com_sun_glass_events_MouseEvent_BUTTON_NONE,
> 0, 0,
> 0, 0,
> 0,
> JNI_FALSE,
> JNI_FALSE);
> CHECK_JNI_EXCEPTION(mainEnv)
> }
> }
> }
>
> Did some tests and it seems to fix the problem.
>
> I do not have permission to make a PR, so posting the fix here. Will
> happily do the PR if required.
>
> Thanks.
>
>