Closed
Bug 612234
Opened 14 years ago
Closed 14 years ago
[gtk2] Menu item rendered with wrong color in open or hover state
Categories
(Core :: Widget: Gtk, defect)
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)
21.78 KB,
image/png
|
Details | |
23.36 KB,
image/png
|
Details | |
31.36 KB,
image/png
|
Details | |
31.11 KB,
image/png
|
Details | |
2.07 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
3.46 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
2.90 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
695 bytes,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
2.84 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
With Clearlooks or High Contrast theme, open menu, the background and/or foreground color of menuitem with open/hover state is wrong.
Comment 4•14 years ago
|
||
I'm hoping this is easy, Karl.
Assignee: nobody → karlt
blocking2.0: ? → final+
Comment 5•14 years ago
|
||
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.
Keywords: regression,
regressionwindow-wanted
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).
Keywords: regressionwindow-wanted
Comment 7•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=df1d1ff6b489&tochange=0f17e5f1eb01
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
The bg color of Clearlook theme issue is caused by http://hg.mozilla.org/mozilla-central/rev/f780f3d3add9
Blocks: 604168
OS: Windows CE → Solaris
Comment 10•14 years ago
|
||
(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?
Assignee | ||
Comment 11•14 years ago
|
||
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.
Assignee | ||
Comment 12•14 years ago
|
||
(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.
Assignee | ||
Comment 13•14 years ago
|
||
Attachment #492289 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 14•14 years ago
|
||
Attachment #492290 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 15•14 years ago
|
||
Attachment #492291 -
Flags: review?(jmuizelaar)
Updated•14 years ago
|
Attachment #492290 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 16•14 years ago
|
||
Comment 17•14 years ago
|
||
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; > }
Assignee | ||
Comment 18•14 years ago
|
||
Attachment #492933 -
Attachment is obsolete: true
Attachment #492947 -
Flags: review?(dao)
Attachment #492933 -
Flags: review?(dao)
Updated•14 years ago
|
Attachment #492947 -
Flags: review?(dao) → review+
Comment 19•14 years ago
|
||
(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?
Comment 20•14 years ago
|
||
Instead of what attachment 492933 [details] [diff] [review] did.
Updated•14 years ago
|
Attachment #492289 -
Flags: review?(jmuizelaar) → review+
Updated•14 years ago
|
Attachment #492291 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 22•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a8cccc6bf4a2 http://hg.mozilla.org/mozilla-central/rev/7ba6c4492ec0 http://hg.mozilla.org/mozilla-central/rev/b0f8b6159c81 http://hg.mozilla.org/mozilla-central/rev/837d7b346a64
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Comment 24•14 years ago
|
||
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?
Assignee | ||
Comment 25•14 years ago
|
||
Just noticed I forgot to add the new file in my last commit.
Attachment #494589 -
Flags: review?(neil)
Comment 26•14 years ago
|
||
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+
Assignee | ||
Comment 27•14 years ago
|
||
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.
Description
•