Closed Bug 1287080 Opened 3 years ago Closed 3 years ago

Move Toolbar, Frame and Gripper widgets to WidgetCache

Categories

(Core :: Widget: Gtk, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: stransky, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Let's move other widgets to WidgetCache and use them in other widget code.
Summary: Move Toolbar widgets to WidgetCache → Move Toolbar, Frame and Gripper widgets to WidgetCache
Attached patch patchSplinter Review
There's the patch, Thanks!
Attachment #8771381 - Flags: review?(andrew)
Comment on attachment 8771381 [details] [diff] [review]
patch

Review of attachment 8771381 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, just a few things:

::: widget/gtk/WidgetStyleCache.cpp
@@ +168,5 @@
>  
>  static GtkWidget*
> +CreateFrameWidget()
> +{
> +  GtkWidget* widget = gtk_frame_new(NULL);

Prefer nullptr.

::: widget/gtk/gtk3drawing.cpp
@@ -972,5 @@
> -    gtk_widget_set_direction(gHandleBoxWidget, direction);
> -
> -    style = gtk_widget_get_style_context(gHandleBoxWidget);
> -    gtk_style_context_save(style);
> -    gtk_style_context_add_class(style, GTK_STYLE_CLASS_GRIP);

It doesn't look like GTK adds this style class to the widget upon initialization, unlike GTK_STYLE_CLASS_TOOLBAR- should we be setting it? GTK 3.4 seems to check for it in gtkthemingengine.c, and sets it on a window's grip.
Attachment #8771381 - Flags: review?(andrew) → review-
Attached patch patch v.2Splinter Review
Good catch, Thanks. There's updated one with GRIPPER style set. It's also checked on 3.20 - we may convert it to CSS nodes later.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=40ccfa60ea9b
Attachment #8771953 - Flags: review?(andrew)
Comment on attachment 8771953 [details] [diff] [review]
patch v.2

Review of attachment 8771953 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks!
Attachment #8771953 - Flags: review?(andrew) → review+
Keywords: checkin-needed
Comment on attachment 8771953 [details] [diff] [review]
patch v.2

Note: check-in a patch from Bug 1287082 before this one.
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4320d509686
Move Toolbar, Frame and Gripper widgets from gtk3drawing to WidgetCache, r=acomminos
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f4320d509686
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.