Move gImageMenuItemWidget/gCheckMenuItemWidget to WidgetCache

RESOLVED FIXED in Firefox 51

Status

()

Core
Widget: Gtk
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: stransky, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
mozilla51
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox50 affected, firefox51 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
+++ 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.
(Reporter)

Comment 1

a year ago
Created attachment 8775533 [details] [diff] [review]
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
Attachment #8775533 - Flags: review?(andrew)
(Reporter)

Updated

a year ago
Blocks: 1290048
(Reporter)

Updated

a year ago
No longer blocks: 1290048
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?
Attachment #8775533 - Flags: review?(andrew) → review+
(Reporter)

Comment 3

a year ago
(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.
(Reporter)

Comment 4

a year ago
Note: This patch needs Bug 1289764 landed before this one.
Keywords: checkin-needed

Comment 5

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/42ddeb0cbb27
Move gImageMenuItemWidget/gCheckMenuItemWidget to WidgetCache, r=acomminos
Keywords: checkin-needed

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/42ddeb0cbb27
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.