Polish mShell/mContainer initialization

RESOLVED FIXED in Firefox 58

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: stransky, Assigned: stransky)

Tracking

Trunk
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 wontfix, firefox58 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Configure mShell/mContainer widget rendering for CSD.
(Assignee)

Comment 2

2 years ago
The question is if we even need this patch when CSD widget setup should be enabled automatically and we get the "csd" style from GdkWindow. I suspect the csd style is not always properly detected but I'll investigate it.
(Assignee)

Updated

2 years ago
Attachment #8921201 - Flags: review?(jhorak)
(Assignee)

Comment 3

2 years ago
Moving to Bug 1399611 as the setup depends on CSD rendering implementation.
Blocks: 1399611
No longer blocks: gtktitlebar
(Assignee)

Updated

2 years ago
Summary: Configure widgets rendering setup for CSD → Polish mShell/mContainer initialization
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8921201 [details]
Bug 1411018 - Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(),

https://reviewboard.mozilla.org/r/192194/#review199530

::: widget/gtk/nsWindow.cpp:3781
(Diff revision 2)
>          gtk_widget_realize(mShell);
>  
> -        // We can't draw directly to top-level window when client side
> -        // decorations are enabled. We use container with GdkWindow instead.
>          GtkStyleContext* style = gtk_widget_get_style_context(mShell);
> -        shellHasCSD = gtk_style_context_has_class(style, "csd");
> +        bool shellHasCSD = gtk_style_context_has_class(style, "csd");

If shellHasCSD can be avoided, please do so by using:

drawToContainer = gtk_style_context_has_class(style, "csd");

and put the following comment above.

::: widget/gtk/nsWindow.cpp:3786
(Diff revision 2)
> -            // Prevent GtkWindow from painting a background to flicker.
> -            gtk_widget_set_app_paintable(mShell, TRUE);
> +         *    Content is rendered to mShell window and we listen
> +         *    Gtk+ events on mShell.

nit: "Content is rendered to mShell window and we listen to the Gtk+ events on mShell."

::: widget/gtk/nsWindow.cpp:3790
(Diff revision 2)
> -            gtk_widget_set_has_window(container, FALSE);
> -            // Prevent GtkWindow from painting a background to flicker.
> -            gtk_widget_set_app_paintable(mShell, TRUE);
> +         * 1) We're running on Gtk+ without client side decorations.
> +         *    Content is rendered to mShell window and we listen
> +         *    Gtk+ events on mShell.
> +         * 2) We're running on Gtk+ > 3.20 and client side decorations
> +         *    are drawn by Gtk+ to mShell. Content is rendered to mContainer,
> +         *    we listen events on mContainer.

nit: "we listen to the events on mContainer"

::: widget/gtk/nsWindow.cpp:3800
(Diff revision 2)
> -        // Set up event widget
> -        eventWidget = shellHasCSD ? container : mShell;
> +#endif
> +        eventWidget = (drawToContainer) ? container : mShell;
> +
>          gtk_widget_add_events(eventWidget, kEvents);
>  
> +        // Prevent GtkWindow from painting a background to flicker.

You mean:
Prevent GtkWindow from painting the background to avoid flickering.
?

::: widget/gtk/nsWindow.cpp:3803
(Diff revision 2)
>          gtk_widget_add_events(eventWidget, kEvents);
>  
> +        // Prevent GtkWindow from painting a background to flicker.
> +        gtk_widget_set_app_paintable(eventWidget, TRUE);
> +
> +        // gtk_container_add() realizes the child widget so we need to

Better:
Before realizing the child widget by gtk_container_add we need to set if container is actually a window or not.
Attachment #8921201 - Flags: review?(jhorak) → review-
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8921201 [details]
Bug 1411018 - Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(),

https://reviewboard.mozilla.org/r/192194/#review199562

Looks good, thanks.
Attachment #8921201 - Flags: review?(jhorak) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5ce1b76adab5
Rename shellHasCSD to drawToContainer and mark our rendering widget by gtk_widget_set_app_paintable(), r=jhorak
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5ce1b76adab5
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.