243372 - [port]GTK2 theme menuitem support from Firefox classic skin

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Ginn Chen]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4.1) Gecko/20040406
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4.1) Gecko/20040406

Port -moz-appearance: menuitem to Mozilla Classic theme

Reproducible: Always
Steps to Reproduce:
1.Choose classic theme
2.Change your native GTK2 theme
3.Restart mozilla, notice the menuitem background, not as good as firefox

Comment 1

[Ginn Chen]

Attachment: Menu.CSS patch

Port some sections from firefox to gtk2
There is no menu.css in global/unix/. Just add to global/win/

Comment 2

[Ginn Chen]

Attachment: a better patch after 237535's checkin

Comment 3

[Ginn Chen]

Attachment: comparison under gnome "simple" theme

Comment 4

[Ginn Chen]

Thanks for the sr=
I'll update the patch because firefox's theme is changing a bit for bug 244955,
bug 174740

Comment 5

[Mike Kaply [:mkaply]]

Comment on attachment 149804 [details] [diff] [review]
a better patch after 237535's checkin

I'm going to give this one to bryner

Comment 6

[Brian Ryner (not reading)]

Comment on attachment 149804 [details] [diff] [review]
a better patch after 237535's checkin

I'm plussing this because I don't see a problem, but in general you need to be
very careful when using the preprocessor on CSS files.	It can strip out id
selectors (starting with #) as comments.  Only if it's at the beginning of the
line though, and I don't see anything in this file that would be affected.

Comment 7

[Ginn Chen]

Yes, I know that.
Create a new theme directory for mozilla/gtk2 would be better.

Comment 8

[[e-mail]]

(In reply to comment #7)
>Create a new theme directory for mozilla/gtk2 would be better.
Which will be simplified once jar.mn preprocessing is in.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

OK, this is not good. the spacing between menu items is too large in mozilla.
this looks rather ugly, especially in comparison to native gtk2 apps, or just to
firefox.

Comment 10

[Doron Rosenberg (IBM)]

Also, in bluecurve, the current menuitem does not change the text color to white
as it does in firefox.

Comment 11

[Doron Rosenberg (IBM)]

This seems to fix the padding problem:

Index: menu.css
===================================================================
RCS file: /cvsroot/mozilla/themes/classic/global/win/menu.css,v
retrieving revision 1.52
diff -u -r1.52 menu.css
--- menu.css    25 Jun 2004 07:42:31 -0000      1.52
+++ menu.css    28 Jun 2004 19:29:58 -0000
@@ -136,9 +136,13 @@
 /* ::::: menu/menuitems in menubar ::::: */
  
 menubar > menu {
+#ifdef MOZ_WIDGET_GTK2
+  padding: 0px 6px 0px 4px;
+#else
   border: 1px solid transparent;
   padding: 2px 5px 2px 7px;
   margin: 1px 0px 1px 0px;
+#endif
 }
  
 menubar > menu[_moz-menuactive="true"] {
@@ -173,7 +177,9 @@
 popup > menu,
 menupopup > menuitem,
 popup > menuitem {
+#ifndef MOZ_WIDGET_GTK2
   padding: 2px;
+#endif
   max-width: 42em;
 }

Comment 12

[Doron Rosenberg (IBM)]

Attachment: Fix padding and menu highlighting

Comment 13

[Ginn Chen]

(In reply to comment #10)
> Also, in bluecurve, the current menuitem does not change the text color to white
> as it does in firefox.

I guess bug 244955 will fix both Firefox and Mozilla for this point. For now,
Mozilla is using gtk2-hovertext, Firefox is using HighlightText, they both does
something wrong with certain theme.
I'll check it later.

Comment 14

[Ginn Chen]

(In reply to comment #10)
> Also, in bluecurve, the current menuitem does not change the text color to white
> as it does in firefox.

HighlightText not work with Gnome-Simple theme, please check bug 244955.

Comment 15

[Frank Wein [:mcsmurf]]

*** Bug 249013 has been marked as a duplicate of this bug. ***

Comment 16

[Doron Rosenberg (IBM)]

What about the padding issue?

Comment 17

[Ginn Chen]

(In reply to comment #16)
> What about the padding issue?

looks good to me, pls get somebody review it.

Comment 18

[Doron Rosenberg (IBM)]

Attachment: Fix padding only by adding ifdefs

Comment 19

[Hideo Oshima]

I tried the latest patch.
It seems good to me too.

Comment 20

[Ginn Chen]

somebody checkin the padding patch please?

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

padding fix checked in
Checking in themes/classic/global/win/menu.css;
/cvsroot/mozilla/themes/classic/global/win/menu.css,v  <--  menu.css
new revision: 1.53; previous revision: 1.52
done

marking fixed again