[Wayland] - Various minor fixes

RESOLVED FIXED in Firefox 59

Status

()

enhancement
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: stransky, Assigned: stransky)

Tracking

(Blocks 1 bug)

Trunk
mozilla59
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(8 attachments, 1 obsolete attachment)

59 bytes, text/x-review-board-request
jhorak
: review+
Details
59 bytes, text/x-review-board-request
jhorak
: review+
Details
59 bytes, text/x-review-board-request
jhorak
: review+
Details
59 bytes, text/x-review-board-request
jhorak
: review+
Details
59 bytes, text/x-review-board-request
jhorak
: review+
Details
59 bytes, text/x-review-board-request
jhorak
: review+
Details
59 bytes, text/x-review-board-request
jhorak
: review+
Details
59 bytes, text/x-review-board-request
jhorak
: review+
Details
The Wayland suport is almost functional, we're missing some minor build fixes and tweaks.
Summary: [Wayland] - Various build fixes → [Wayland] - Various minor fixes
Comment on attachment 8942116 [details]
Bug 1430018 - Don't use nsWindow::IsX11Display() as it's not implemented,

https://reviewboard.mozilla.org/r/212372/#review218486
Attachment #8942116 - Flags: review?(jhorak) → review+
Comment on attachment 8942117 [details]
Bug 1430018 - Add runnable function name to NewRunnableFunction() calls,

https://reviewboard.mozilla.org/r/212374/#review218488
Attachment #8942117 - Flags: review?(jhorak) → review+
Comment on attachment 8942119 [details]
Bug 1430018 - Don't use gdk_x11_screen_supports_net_wm_hint(_NET_WM_STATE_FULLSCREEN) on Wayland,

https://reviewboard.mozilla.org/r/212378/#review218492

::: widget/gtk/nsWindow.cpp:5088
(Diff revision 1)
>                                 deskBounds.width, deskBounds.height,
>                                 getter_AddRefs(screen));
>    return screen.forget();
>  }
>  
> +#ifdef MOZ_X11

Please don't move the ifdefs there.

::: widget/gtk/nsWindow.cpp:5107
(Diff revision 1)
>  nsWindow::MakeFullScreen(bool aFullScreen, nsIScreen* aTargetScreen)
>  {
>      LOG(("nsWindow::MakeFullScreen [%p] aFullScreen %d\n",
>           (void *)this, aFullScreen));
>  
> -    if (!IsFullscreenSupported(mShell)) {
> +#ifdef MOZ_X11

I would not add another #ifdef there. Just make sure the mIsX11Display is set before calling IsFullscreenSupported (like proposed already).

Please add brief comment why checking for mIsX11Display there (like in Wayland fullscreen is always supported).

Giving r+ with that fixed.
Attachment #8942119 - Flags: review?(jhorak) → review+
Comment on attachment 8942120 [details]
Bug 1430018 - Block popup input events on container widget for popups on Wayland,

https://reviewboard.mozilla.org/r/212380/#review218500

By discussion, we've agreed that this change is no longer required since eventWidget was introduced.
Attachment #8942120 - Flags: review?(jhorak)
Comment on attachment 8942121 [details]
Bug 1430018 - Don't call gdk_flush() from expose event on Wayland as it crashes Gtk+ (Gnome BZ #773307),

https://reviewboard.mozilla.org/r/212382/#review218504
Attachment #8942121 - Flags: review?(jhorak) → review+
Comment on attachment 8942146 [details]
Bug 1430018 - Fixed wrong profile param name at DBusRemoteClient::DoSendDBusCommandLine(),

https://reviewboard.mozilla.org/r/212408/#review218506
Attachment #8942146 - Flags: review?(jhorak) → review+
Comment on attachment 8942147 [details]
Bug 1430018 - dlsym gdk_wayland_* routines as those are available at Gtk+ > 3.8,

https://reviewboard.mozilla.org/r/212410/#review218512
Attachment #8942147 - Flags: review?(jhorak) → review+
Comment on attachment 8942145 [details]
Bug 1430018 - Add namespace mozilla::widget to nsNativeThemeGTK.cpp as it's needed by ScreenHelperGTK,

https://reviewboard.mozilla.org/r/212406/#review218514
Attachment #8942145 - Flags: review?(jhorak) → review+
Comment on attachment 8942118 [details]
Bug 1430018 - Unify GtkCompositorWidgetInitData() call for X11 and Wayland,

https://reviewboard.mozilla.org/r/212376/#review218516

::: widget/gtk/nsWindow.cpp:7108
(Diff revision 1)
> -                                  GetClientSize());
> -  } else
> -#endif
> -  {
> -    *aInitData = mozilla::widget::GtkCompositorWidgetInitData(
> +  *aInitData = mozilla::widget::GtkCompositorWidgetInitData(
> -                                  mXWindow,
> +                                (mXWindow != X11None) ? mXWindow : (uintptr_t)nullptr,

Looks much better without ifdefs, could you please use
just empty nsCString() without nullptr, like:

mXDisplay ? nsCString(XDisplayString(mXDisplay)) : nsCString()

as long as I don't see nsCString(nullptr) anywhere in the sources and it looks strange to me.

r+ with that fixed.
Attachment #8942118 - Flags: review?(jhorak) → review+
Attachment #8942120 - Attachment is obsolete: true
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/509364918089
Don't use nsWindow::IsX11Display() as it's not implemented, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/15f4b0d4bae5
Add runnable function name to NewRunnableFunction() calls, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/4cf0168d93c5
Unify GtkCompositorWidgetInitData() call for X11 and Wayland, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/7350873319fa
Don't use gdk_x11_screen_supports_net_wm_hint(_NET_WM_STATE_FULLSCREEN) on Wayland, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/cb71ca2516f2
Don't call gdk_flush() from expose event on Wayland as it crashes Gtk+ (Gnome BZ #773307), r=jhorak
https://hg.mozilla.org/integration/autoland/rev/c361754494d0
Add namespace mozilla::widget to nsNativeThemeGTK.cpp as it's needed by ScreenHelperGTK, r=jhorak
https://hg.mozilla.org/integration/autoland/rev/e432cc393634
Fixed wrong profile param name at DBusRemoteClient::DoSendDBusCommandLine(), r=jhorak
https://hg.mozilla.org/integration/autoland/rev/40a2e98240cf
dlsym gdk_wayland_* routines as those are available at Gtk+ > 3.8, r=jhorak
You need to log in before you can comment on or make changes to this bug.