Bug 183143 – [GTK] testRunner.setWindowIsKey() has no effect on the web process side in WebKit2

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Daniel Bates]

The GTK implementation of testRunner.setWindowIsKey() only updates a boolean flag and does not change the activation state of the window. We need to implement so as to be able to test page activation issues Once we implement such support we may be able to unskip some or all of the following tests:

fast/dom/Window/window-focus-self.html
fast/events/blur-focus-window-should-blur-focus-element.html
fast/selectors/querySelector-window-inactive.html
fast/selectors/selection-window-inactive.html
scrollbars/corner-resizer-window-inactive.html

[Michael Catanzaro]

Thanks for reporting a bug so that we know about the problem.

Implementing setWindowIsKey to focus the window should be easy; we just need to call gtk_window_present(). *This function is broken on Wayland* so developers and test expectations will need to take this into account.
 
Claudio pointed out one problem: there is no inverse function. To un-focus the window, we'll need to create a second window and give that window focus. We might have to keep it around forever, too, to prevent the original window from regaining focus. That's quite non-ideal, but I don't see a better way.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #1)
> Thanks for reporting a bug so that we know about the problem.
> 
> Implementing setWindowIsKey to focus the window should be easy; we just need
> to call gtk_window_present(). *This function is broken on Wayland* so
> developers and test expectations will need to take this into account.

To be clear, the failure expectations should move from platform/gtk/TestExpectations to platform/gtk-wayland/TestExpectations.

[Claudio Saavedra]

I've been toying with having a secondary window for this purpose and at least 4 of those tests are fixed that way. The one that is not fixed seems to be related to some selector getting the focus instead of the window, so I still need to investigate that a bit further.

[Claudio Saavedra]

These two tests, now broken, would also pass.

fast/selectors/text-field-selection-text-shadow.html
fast/selectors/text-field-selection-stroke-color.html

[Claudio Saavedra]

Created attachment 334891 [details]
Patch

[Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 334891 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334891&action=review

> LayoutTests/platform/gtk/TestExpectations:2386
>  webkit.org/b/183143 fast/events/blur-focus-window-should-blur-focus-element.html

Please file a new bug for this one?

[Carlos Garcia Campos]

Comment on attachment 334891 [details]
Patch

Could we create the second window on demand?

[Claudio Saavedra]

Created attachment 334894 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 334894 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334894&action=review

> Tools/WebKitTestRunner/PlatformWebView.h:118
> +    GtkWidget *m_otherWindow;

{ nullptr };

> Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp:45
> +    , m_otherWindow(nullptr)

Adn we don't need this here.

[Claudio Saavedra]

Created attachment 334896 [details]
Patch for landing

[Claudio Saavedra]

(In reply to Ms2ger from comment #6)
> Comment on attachment 334891 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334891&action=review
> 
> > LayoutTests/platform/gtk/TestExpectations:2386
> >  webkit.org/b/183143 fast/events/blur-focus-window-should-blur-focus-element.html
> 
> Please file a new bug for this one?

I was thinking to keep this one open since I don't understand yet well what is going on in that test that is causing it to fail.

[Michael Catanzaro]

Comment on attachment 334896 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=334896&action=review

> Tools/WebKitTestRunner/PlatformWebView.h:118
> +    GtkWidget *m_otherWindow { nullptr };

Nit: GtkWidget* m_otherWindow

> LayoutTests/platform/gtk/TestExpectations:2386
>  webkit.org/b/183143 fast/events/blur-focus-window-should-blur-focus-element.html

Please report a new bug for this, and move it to the normal failures section of the expectations.

Also: I'm pretty sure all these tests will still be broken in Wayland, as mentioned above, so the expectations should probably be moved to the Wayland expectations?

[Michael Catanzaro]

Good job btw: I was too scared to try using a spare GtkWindow, but seems it worked out fine.

[Claudio Saavedra]

Created attachment 334999 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 334999 [details]
Patch for landing

Clearing flags on attachment: 334999

Committed r229283: <https://trac.webkit.org/changeset/229283>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/38138918>

[Claudio Saavedra]

(In reply to Michael Catanzaro from comment #12)
> 
> Please report a new bug for this, and move it to the normal failures section
> of the expectations.

https://bugs.webkit.org/show_bug.cgi?id=183328