Closed Bug 612234 Opened 9 years ago Closed 9 years ago

[gtk2] Menu item rendered with wrong color in open or hover state

Categories

(Core :: Widget: Gtk, defect)

x86
Solaris
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- final+

People

(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)

References

Details

(Keywords: regression)

Attachments

(9 files, 1 obsolete file)

With Clearlooks or High Contrast theme, open menu, the background and/or foreground color of menuitem with open/hover state is wrong.
blocking2.0: --- → ?
I'm hoping this is easy, Karl.
Assignee: nobody → karlt
blocking2.0: ? → final+
I'm not reproducing the missing menu item in attachment 490539 [details] (Clearlooks), nor am I reproducing the wrong bg color on the open menu title.
However, I do see the wrong fg color on the open menu title.
Whiteboard: regression
(In reply to comment #5)
> I'm not reproducing the missing menu item in attachment 490539 [details] (Clearlooks),
> nor am I reproducing the wrong bg color on the open menu title.

Perhaps the theme on Solaris is a little different.

> However, I do see the wrong fg color on the open menu title.

Just found it is a recent regression.
The regression window is between changeset 57199 (Nov 10 nightly) to changeset 57324 (Nov 11 nightly).
The fg color and bg color issue are caused by different commits.

The fg color issue with High Contrast theme is caused by
http://hg.mozilla.org/mozilla-central/rev/4563259585a2

Now I'm trying to find out the regression window of the bg color issue with Clearlook theme.
OS: Linux → Windows CE
Blocks: 609422
The bg color of Clearlook theme issue is caused by
http://hg.mozilla.org/mozilla-central/rev/f780f3d3add9
Blocks: 604168
OS: Windows CE → Solaris
(In reply to comment #8)
> The fg color and bg color issue are caused by different commits.
> 
> The fg color issue with High Contrast theme is caused by
> http://hg.mozilla.org/mozilla-central/rev/4563259585a2
> 
> Now I'm trying to find out the regression window of the bg color issue with
> Clearlook theme.

Not sure why that commit would cause the issue as presumably the File menu element is not disabled at that point?
Ian,

I guess it is because menubar > menu[_moz-menuactive="true"]:not([disabled="true"]) is more specific than menubar > menu[open].

Before your patch, the color for menuitem should be

1) Normal,
75 menubar > menu {
76   padding: 0px 4px;
77   color: -moz-menubartext;
78 }

2) Mouse on menu but not dropdown the menu
89 menubar > menu[_moz-menuactive="true"] {
90   color: -moz-menubartext;
91 }

3) Menu is dropdown
93 menubar > menu[open] {
94   color: -moz-menubarhovertext;
95   background-color: -moz-menuhover;
96 }

Because you're adding another rule for condition 2), it is more specific than condition 3).

You can add "! important" or ":not([disabled="true"]) " for condition 3) to fix this problem.
(In reply to comment #9)
> The bg color of Clearlook theme issue is caused by
> http://hg.mozilla.org/mozilla-central/rev/f780f3d3add9

This is caused by a series of bugs.
1) In Bug 598561 Don't rename pixman functions when system pixman is in use, we didn't define MOZ_TREE_PIXMAN, therefore we didn't rename pixman functions even with our internal pixman library.
2) We didn't rename all pixman export functions
3) Although in pixman.h, it doesn't have PIXMAN_EXPORT, but for Sun Studio compiler, pixman functions are still compiled as global scope.

With these bugs and a certain combination of system cairo, pixman library and mozilla internal pixman library, we got the wrong background.
Attachment #492289 - Flags: review?(jmuizelaar)
Attachment #492290 - Flags: review?(jmuizelaar)
Attachment #492291 - Flags: review?(jmuizelaar)
Attachment #492290 - Flags: review?(jmuizelaar) → review+
Attached patch fix the fg color of menu (obsolete) — Splinter Review
Assignee: karlt → ginn.chen
Status: NEW → ASSIGNED
Attachment #492933 - Flags: review?(dao)
Comment on attachment 492933 [details] [diff] [review]
fix the fg color of menu

>diff --git a/toolkit/themes/gnomestripe/global/menu.css b/toolkit/themes/gnomestripe/global/menu.css
>--- a/toolkit/themes/gnomestripe/global/menu.css
>+++ b/toolkit/themes/gnomestripe/global/menu.css
>@@ -85,17 +85,17 @@ menubar:-moz-lwtheme > menu:not([open="t
>   color: inherit;
>   text-shadow: inherit;
> }
> 
> menubar > menu[_moz-menuactive="true"]:not([disabled="true"]) {

Can you add :not([open]) here? And :not(:-moz-lwtheme) while you're at it?

>   color: -moz-menubartext;
> }
> 
>-menubar > menu[open] {
>+menubar > menu[open]:not([disabled="true"]) {
>   color: -moz-menubarhovertext;
>   background-color: -moz-menuhover;
> }
Attachment #492933 - Attachment is obsolete: true
Attachment #492947 - Flags: review?(dao)
Attachment #492933 - Flags: review?(dao)
Attachment #492947 - Flags: review?(dao) → review+
(In reply to comment #17)
> Comment on attachment 492933 [details] [diff] [review]
> fix the fg color of menu
> 
> >diff --git a/toolkit/themes/gnomestripe/global/menu.css b/toolkit/themes/gnomestripe/global/menu.css
> >--- a/toolkit/themes/gnomestripe/global/menu.css
> >+++ b/toolkit/themes/gnomestripe/global/menu.css
> >@@ -85,17 +85,17 @@ menubar:-moz-lwtheme > menu:not([open="t
> >   color: inherit;
> >   text-shadow: inherit;
> > }
> > 
> > menubar > menu[_moz-menuactive="true"]:not([disabled="true"]) {
> 
> Can you add :not([open]) here? And :not(:-moz-lwtheme) while you're at it?
> 
Did you mean as well as or instead of?
Duplicate of this bug: 611739
Attachment #492289 - Flags: review?(jmuizelaar) → review+
Attachment #492291 - Flags: review?(jmuizelaar) → review+
Duplicate of this bug: 615376
Comment on attachment 492291 [details] [diff] [review]
fix issue 3) in comment #12

>+/* In libxul builds we don't ever want to export pixman symbols */
>+#ifdef cairo_public
>+#   define PIXMAN_EXPORT cairo_public
This broke my non-libxul build where cairo_public defines to extern __declspec(dllexport) but PIXMAN_EXPORT used to define to nothing.
Should this say #ifdef MOZ_ENABLE_LIBXUL instead?
Just noticed I forgot to add the new file in my last commit.
Attachment #494589 - Flags: review?(neil)
Comment on attachment 494589 [details] [diff] [review]
fix non libxul build

I can confirm that this patch fixes my Windows non-libxul build.
[No idea as to its effect on any other sort of build though.]

>-		*n++ = bswap_32 (*d++);
>+		*n++ = bswap_32 (*d);
>+        d++;
[Not intended to be part of this patch I take it!]
Attachment #494589 - Flags: review?(neil) → review+
Fix for non libxul build and add the missing patch file
http://hg.mozilla.org/mozilla-central/rev/bfec67c46b7c
You need to log in before you can comment on or make changes to this bug.