1289763 - Move gImageMenuItemWidget/gCheckMenuItemWidget to WidgetCache

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

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

+++ This bug was initially created as a clone of Bug #1288413 +++

Let's move other widgets to WidgetCache and use them in other widget code.

Comment 1

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

Attachment: patch

There's a patch for it. I used a similar mechanism as for plain radio/checkboxes, the menuitem container has _CONTAINER widget ID now.

There are not NS_THEME_CHECKMENUITEM_CONTAINER/NS_THEME_RADIOMENUITEM_CONTAINER classes so the _CONTAINER widgets are used only internally and are not exported by nsNativeThemeGTK::GetGtkWidgetAndState().

We also need render frame/background on 3.20.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fcb85a74898c

Comment 2

[Andrew Comminos [:acomminos]]

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

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

Looks good to me! Just one question;

::: widget/gtk/WidgetStyleCache.cpp
@@ +533,5 @@
> +      break;
> +    case MOZ_GTK_CHECKMENUITEM:
> +      style = CreateChildCSSNode(GTK_STYLE_CLASS_CHECK,
> +                                 MOZ_GTK_CHECKMENUITEM_CONTAINER);
> +      break;

GTK docs say the "radio" and "check" subnodes should have either the ".left" or ".right" style class. Should we set these?

Comment 3

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

(In reply to Andrew Comminos [:acomminos] from comment #2)
> Comment on attachment 8775533 [details] [diff] [review]
> patch
> 
> Review of attachment 8775533 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me! Just one question;
> 
> ::: widget/gtk/WidgetStyleCache.cpp
> @@ +533,5 @@
> > +      break;
> > +    case MOZ_GTK_CHECKMENUITEM:
> > +      style = CreateChildCSSNode(GTK_STYLE_CLASS_CHECK,
> > +                                 MOZ_GTK_CHECKMENUITEM_CONTAINER);
> > +      break;
> 
> GTK docs say the "radio" and "check" subnodes should have either the ".left"
> or ".right" style class. Should we set these?

That's derived from widget direction in gtk_check_menu_item_direction_changed() so we don't need to set it by extra class.

Comment 4

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

Note: This patch needs Bug 1289764 landed before this one.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/42ddeb0cbb27
Move gImageMenuItemWidget/gCheckMenuItemWidget to WidgetCache, r=acomminos

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/42ddeb0cbb27