244955 - [GNOME] incorrect system colors for the menu item text

Status: VERIFIED FIXED

(Firefox :: Shell Integration, 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

HighlightText should use gtk2 style for text


Reproducible: Always
Steps to Reproduce:
try GNOME Theme "Simple",



Actual Results:  
When menuitem is active, the text will be white on white background.

Expected Results:  
Menuitem fg color should be text selected fg color.

Comment 1

[Ginn Chen]

Some thing like bug 232102,

We should do the same thing to HighLightText.

Comment 2

[Ginn Chen]

Attachment: patch

Comment 3

[Mike Connor [:mconnor]]

blizzard/bryner would be the people to ping for review here, pch is on hiatus.

Comment 4

[Ali Ebrahim]

Is this even a Firefox bug? You're patching
/cvsroot/mozilla/widget/src/gtk2/nsLookAndFeel.cpp , and might need review from
someone else, not a Firefox peer.

Comment 5

[Mike Connor [:mconnor]]

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

Comment 6

[Ginn Chen]

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

I'm patching gtk2/nsLookAndFeel.cpp becuase Firefox and Mozilla have the same
problem. Their menu fg color is wrong. Firefox's menu bg is right because
118296 changed its skin. Mozilla's menu bg is wrong, we need bug 243372 to fix
Mozilla's theme too.

Comment 7

[Ginn Chen]

Attachment: patch (this one is better after 237535's checkin)

Comment 8

[Ginn Chen]

Attachment: Comparision(gedit, firefox, patched firefox under Gnome Simple Theme)

Comment 9

[Ginn Chen]

Attachment: Patch v2.0

Create a new keyword for menu hover text, because fg[PRELIGHT] for menuitem may
differ from buttons, toolbarbuttons etc.

Currently some themes, e.g. Crux, Mist, have fg[PRELIGHT] setting for menuitem
style in their "gtk2.0/gtkrc". We should not use fg[SELECTED] for menu item
text in gtk2.0 build, because we don't use bg[SELECTED] for menu item
background in Firefox gtk2 build already.
This patch is helping on this.

Comment 10

[Mike Connor [:mconnor]]

this patch is probably going to be bitrotted by the new theme that's getting
checked in this week (already on aviary branch), looks good, though I think it
perhaps should get review from a gfx peer (which I'm not)

Comment 11

[Ginn Chen]

Comment on attachment 150188 [details] [diff] [review]
Patch v2.0

oops, I forgot following in this patch.
     case eColor_highlighttext:
	 // text of selected item
-	 aColor = GDK_COLOR_TO_NS_RGB(mStyle->fg[GTK_STATE_SELECTED]);
+	 aColor = GDK_COLOR_TO_NS_RGB(mStyle->text[GTK_STATE_SELECTED]);
	 break;

it's in first patch, unobsolete it.

Comment 12

[Peto]

No, the bug isn't bitrotted by the new theme. The menu text is still white
(invisible) with Firefox 0.9 stable on GNOME 2.2

Comment 13

[Ginn Chen]

(In reply to comment #12)
> No, the bug isn't bitrotted by the new theme. The menu text is still white
> (invisible) with Firefox 0.9 stable on GNOME 2.2
> 
1. Patch Firefox
2. Rebuild Firefox with --enable-default-tookit=gtk2
3. After change GNOME Theme, restart Firefox.

Comment 14

[Mike Connor [:mconnor]]

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

Comment 15

[Ginn Chen]

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

bryner, did your sr= contain this patch?
This will fix the second row of my screenshot. It should be in patch_v2.0.
Sorry for that. Can it be checked in?

Comment 16

[Ginn Chen]

Attachment: patch v3

1.Merge last two patches.
2.Firefox Theme directory changed, re-do it.
3.Do patch to Mozilla, because bug 243372 checked it.

Comment 17

[Ginn Chen]

Comment on attachment 151670 [details] [diff] [review]
patch v3

bryner, I'll remove #ifdef in gnomestripe/global/menu.css,
because it is gtk2 only. Thanks for your advice.

Comment 18

[Eric Dorland]

What's the status of this patch? Is it a working fix for this problem?

Comment 19

[Ginn Chen]

I think it will work for most themes, unless the theme-engine hardcode colors in
its cpp code.
Wait for review. less change from v2 to v3.

Comment 20

[Raul Acevedo]

Will this fix make it into the next firefox release?

Comment 21

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

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

Comment 22

[Ginn Chen]

tested, this patch works well with RH9 bluecurve

Comment 23

[Ginn Chen]

Comment on attachment 151670 [details] [diff] [review]
patch v3

hold on, until bug 251492 fix the CSS naming.

Comment 24

[Ginn Chen]

dbaron, I need color name for menuhovertext/menuhoverbackground.

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

How about MenuHoverText and MenuHover?

Comment 26

[Ginn Chen]

a new patch posted on bug 251492

Comment 27

[chris hofmann]

Comment on attachment 151670 [details] [diff] [review]
patch v3

lets get reviews

Comment 28

[Ginn Chen]

Comment on attachment 151670 [details] [diff] [review]
patch v3

As dbaron's suggestion, the patch in bug 244955 is more reasonable than this
one.
The difference is only the naming pair.

Comment 29

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 151670 [details] [diff] [review]
patch v3

review-, for reasons I've stated in one of the other bugs, and because the
patch is obsoleted by a patch elsewhere

Comment 30

[chris hofmann]

ok, minus for 1.0.  

Comment 31

[Ginn Chen]

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

Comment 32

[mey.wer]

(In reply to comment #30)
> ok, minus for 1.0.  
??

I just want you to know, that using 
Firefox version 1.0 preview release 
[Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20040913 Firefox/0.10] 
(got as suse 9.0 rpm) 
and gnome theme simple (schlicht) (got as sus 9.0 rpms) I've still this problem,
that marked (chosen) "lines" in menus are "invisible" (white text on white
background)

Same problem occurs in thunderbird 0.8.

(reproducible always)

Comment 33

[Ginn Chen]

Sorry, here was a typo. should be bug 251492

(In reply to comment #28)
> (From update of attachment 151670 [details] [diff] [review])
> As dbaron's suggestion, the patch in bug 244955 is more reasonable than this
                                           ^^^^^^ 
> one.
> The difference is only the naming pair.
> 

Comment 34

[Ginn Chen]

Fixed by bug 251492

Comment 35

[Brian Ryner (not reading)]

Comment on attachment 151670 [details] [diff] [review]
patch v3

removing obsolete review request

Comment 36

[Kevin Buhr]

The original bug was for white-on-white highlighted menu text on Firefox using
the GTK2 "Simple" theme.  I can verify that this problem still exists on an
Aviary branch build pulled on August 18, so I'm reopening until we can backport
the patch from bug 251492.

Switched component: OS Integration -> Menus (though that's kind of silly place
to put it, too).

Comment 37

[Kevin Buhr]

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

Comment 38

[Mike Connor [:mconnor]]

The aviary branch is current taking only security fixes and regressions from
security fixes.  If vendors wish to backport this to that branch, that is at
their own discretion.  Otherwise, the fix will be in Firefox/Thunderbird 1.5. 
FIXED is used for trunk checkins, we use 

As for the category, this shouldn't be in Firefox at all, its a widget bug, but
mucking with long-dead bugs is bad, so I'm just undoing the bogus changes.

Comment 39

[Kevin Buhr]

Oops, sorry for making a mess.  If someone looking for a 1.0.x fix winds up
here, I've done a (GTK2-only) backport for Debian that may be useful:

     http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=257430#msg90

Comment 40

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #39)
> Oops, sorry for making a mess.  If someone looking for a 1.0.x fix winds up
> here, I've done a (GTK2-only) backport for Debian that may be useful:

This fix seems to be very useful, especially for debian packages. I'm using the
debian package of Firefox 1.0.6 and it shows the menu colors correctly. All
other builds of the 1.8 branch or 1.9 trunk make the highlighted menutext
unreadable. The black color of menutext stays when hovering a menuitem. It seems
that using "color: -moz-menuhovertext" doesn't work. Instead "background-color:
-moz-menuhover;" is fine and highlight the background.

Changing the color of a highlighted menuitem to HighlightText also fixes the
issue. But I think we should take a look at the debian part fix?

Comment 41

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Hovering a menu item doesn't change highlighted color

Comment 42

[Ginn Chen]

Please do not reopen this bug again.
This will never be fixed in Firefox 1.0.x. (See comment #36)

Also debian's back porting should not be discussed here.

Comment 43

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #42)
> Please do not reopen this bug again.
> This will never be fixed in Firefox 1.0.x. (See comment #36)
> 
> Also debian's back porting should not be discussed here.

Ginn, I never told that we should fix that for Fx 1.0.x. The issue still occurs
in current 1.8 branch and 1.9 trunk builds! There we should fix it, because it's
a major issue. Should I open a new bug for that?

Comment 44

[Ginn Chen]

(In reply to comment #43)
 > Ginn, I never told that we should fix that for Fx 1.0.x. The issue still occurs
> in current 1.8 branch and 1.9 trunk builds! There we should fix it, because it's
> a major issue. Should I open a new bug for that?

I thought it was fixed. Does other GNOME application have the same problem?
Maybe the Simple theme intends to it, I'm not sure, I'll check on Monday.

Comment 45

[Ginn Chen]

Attachment: firefox nightly on debian ppc

I just tested on debian/ppc. It is fine.
I also tested seamonkey with Simple theme on JDS3/x86. The result is same.

Comment 46

[Henrik Skupin [:whimboo][⌚️UTC+2]]

> I thought it was fixed. Does other GNOME application have the same problem?
> Maybe the Simple theme intends to it, I'm not sure, I'll check on Monday.

You are right. There is something wrong by using gnome applications under KDE.
Also the simple gnome theme shows that issue. Using another gnome theme solves it. 

This bug is really fixed. Thank you.