Martin Stránský [:stransky] (ni? me) Assignee
	Description • 11 years ago

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

GTK+ 3.0 and GNOME 3 are approaching and we should get Firefox ready for them. This bug is for mapping fixes in menu widgets.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 1 • 11 years ago

There are two issues in menus, in the recent code. The item sizes (they're so small) and bad rendering of the menu background - there are artifacts after mouse over.

	Karl Tomlinson (:karlt)
	Comment 3 • 11 years ago

Comment on attachment 767088 [details] [diff] [review]
menu padding patch

Interesting.  horizontal-padding usage was removed sometime after GTK+ 3.6.3.

The only thing that really doesn't look right to me is
moz_gtk_menu_arrow_paint().  I think the rect here is inside something that is
-moz-appearance: menuitem, and so border_width and padding have already been
removed.  Also, we can't really change the arrow size by the scaling here, or
it may draw outside the rect, leaving artifacts.

I'm guessing the relevant markup is
http://hg.mozilla.org/mozilla-central/annotate/3b955f306226/toolkit/content/widgets/menu.xml#l185
http://hg.mozilla.org/mozilla-central/annotate/3b955f306226/toolkit/themes/linux/global/menu.css#l126

I suspect the right approach for the MENUARROW is to implement
GetMinimumWidgetSize() and perhaps change the sizes specified in CSS.

I suggest dealing with arrow changes in a separate patch as they are more
complicated and the rest of the changes are looking good.

>+    if (direction == GTK_TEXT_DIR_RTL) {
>+        x = (rect->width - indicator_size - offset - horizontal_padding);
>+    }
>+    else {
>+        x = (rect->x + offset + horizontal_padding);
>+    }

Remove the unnecessary parentheses, please.

>-    y = rect->y + (rect->height - indicator_size) / 2;

>+    y = (rect->height - indicator_size) / 2;

I expect rect->y is needed here, so no change, I assume.

>     if (wide_separators)
>-        *size = separator_height + border.top;
>+        *size = separator_height + padding.top + border_width*2;
>     else
>-        *size = border.top + border.bottom;
>+        *size = padding.top + padding.bottom + border_width;

Looking at gtk_menu_item_real_get_height, I expect this to always include
padding.top + padding.bottom + border_width * 2.  Then there's
separator_height for wide_separators and 1 for the else.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 4 • 11 years ago

Thanks! There's an updated one, without the the arrow changes.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 5 • 11 years ago

btw. The menu also needs clipping, the selected (active) menuitem background is drawn over the whole popup menu.

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e4ea66e40c7

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/4e4ea66e40c7

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 8 • 11 years ago

Menu seems to be fine now. The clipping will be addressed in Bug 877606.