1319066 - [GtkTooltip] Use WidgetCache to get colors at nsLookAndFeel

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[Martin Stránský [:stransky] (ni? me)]

Transfer GtkTooltip to the widget cache.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Attachment: patch

Comment 2

[Karl Tomlinson (:karlt)]

Comment on attachment 8812750 [details] [diff] [review]
patch

>+    case MOZ_GTK_TOOLTIP_BOX:
>+      style = CreateChildCSSNode("box",
>+                                 MOZ_GTK_TOOLTIP);
>+      break;
>+    case MOZ_GTK_TOOLTIP_BOX_LABEL:
>+      style = CreateChildCSSNode("label",
>+                                 MOZ_GTK_TOOLTIP_BOX);

This is losing the "horizontal" class on the box, which can be observed by
adding "tooltip box.horizontal {padding: 100px;}" to gtk.css, for example.

However, rather than having to work out which classes should be on each
context, please continue to use the CreateStyleForWidget() approach for root
nodes, where possible, and it is possible here.  That automatically finds the
correct classes for us and means the same code can be used for pre/post 3.20
GTK versions.

The best way to do this I think is to move MOZ_GTK_TOOLTIP and its child
widget contexts to GetWidgetRootStyle().  That probably requires a version
test for the MOZ_GTK_TOOLTIP case, but I expect that node is a rare exception.

The box and label are each actually root nodes of separate widgets, not child
gadgets within the GtkTooltipWindow.  Perhaps the only difference is that GTK
gives the nodes an object type, and so gtk_style_context_get_style() can find
class style properties.  (gtkcssmatcher.c prefers the object name to the type,
and so the type is not used for matching when there is a name.)  Although
Gecko doesn't call gtk_style_context_get_style() on these particular contexts,
I'd prefer to follow the same CreateStyleForWidget() pattern for all widget
root nodes where that is possible.

The changes to the other files all look good, thanks.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Attachment: v.2 patch

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 8813268 [details] [diff] [review]
v.2 patch

>+    case MOZ_GTK_TOOLTIP:
>+    case MOZ_GTK_TOOLTIP_BOX:
>+    case MOZ_GTK_TOOLTIP_BOX_LABEL: {

This works, but are you intentionally avoiding the pattern used for other
widgets in this function?:

    case MOZ_GTK_TOOLTIP_BOX_LABEL:
      style = CreateStyleForWidget(gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 0),
                                   MOZ_GTK_TOOLTIP);
      break;

which essentially removes node-specific sStyleStorage[] use/manipulation.

>+    ReleaseStyleContext(boxStyle);
> 
>     // Label drawing
>     InsetByBorderPadding(&rect, boxStyle);

Please move ReleaseStyleContext() to after InsetByBorderPadding(), as boxStyle
is used there also.  r+ with this change.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Attachment: patch v.3 for check-in

Ah, Thanks. Yes I think it's better to follow the hierarchy scheme here.

Comment 6

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8813569 [details] [diff] [review]
patch v.3 for check-in

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee6c42a4e313386c860b20962ec3565fd0dda565

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8813569 [details] [diff] [review]
patch v.3 for check-in

Thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ba00502fa16
Move complete GtkTooltip stack to WidgetCache. r=karlt

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3ba00502fa16