Menu Hover Displays Incorrectly when Win XP Appearance Theme is Changed

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
11 years ago
7 years ago

People

(Reporter: Portfolioso, Unassigned)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre

When hovering over the men

Reproducible: Always

Steps to Reproduce:
1. Leave Minefield open
2. Change the Windows XP Appearance (display properties... appearance tab... Windows and buttons (switch from Windows classic to Windows XP theme or vice versa)
3. Mouseover the File/Edit/View/History etc) on the top menu and notice how it displays incorrectly
Actual Results:  
When you mouseover the menuitems instead of getting a normal menu behavior, it is displayed incorrectly.

Expected Results:  
The menus should behave normally

Screenshots:
Switching from XP Normal to Windows Classic: http://img514.imageshack.us/img514/3568/umnl4.png

Switching from XP Classic to XP Normal: 
http://img370.imageshack.us/img370/4693/um2mr9.png
(Reporter)

Updated

11 years ago
Status: UNCONFIRMED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → INVALID
(Reporter)

Updated

11 years ago
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---

Updated

10 years ago
Status: UNCONFIRMED → NEW
Component: Menus → Themes
Ever confirmed: true
Product: Firefox → Core
QA Contact: menus → themes
Version: unspecified → Trunk

Updated

10 years ago
Blocks: 436779
(Assignee)

Updated

10 years ago
Product: Core → SeaMonkey

Updated

10 years ago
Component: Themes → Widget: Win32
Product: SeaMonkey → Core

Comment 1

10 years ago
I think that this is a timing problem.  Prior to the Windows theme consolidation, the background color and beveling was determined by nsNativeThemeWin, and the text color was determined by nsLookAndFeel.  NTW updates the flat menu status when it gets a theme change notification, whereas LAF checks the flat menu status when the text color is requested.  It seems that Windows does not report the correct flat menu status when it sends out a theme change notification, but it does report the correct flat menu status later on, which is why we have white text on a gray background or black text on a blue background.

With the recent unification where both NTW and LAF are querying the flat menu status when notified of a theme change, we do longer get that color mismatch and the poor readability resulting from that, but the menus are still wrong.

Fixing this will require that we decouple the flat menu checking from the theme change invalidation, replacing the sFlatMenus variable with a function that checks the current flat menu status (or, if SystemParametersInfo is too expensive, we can try to cache it, but this on-the-fly checking is what we used to do for menu text color)...

Comment 2

10 years ago
Created attachment 331965 [details] [diff] [review]
patch

This fixes it.  But is this edge case worth the trouble?
Assignee: nobody → kliu
Status: NEW → ASSIGNED
Attachment #331965 - Flags: superreview?(vladimir)
Attachment #331965 - Flags: review?(vladimir)
(In reply to comment #2)
> Created an attachment (id=331965) [details]
> patch
> 
> This fixes it.  But is this edge case worth the trouble?

Patch looks ok to me, but would it be fixed if we were to just set a 'check menu flatness' flag or something for the next time a menu is drawn, and then have UseFlatMenus still cache the value?  Seems like that might be the best of both worlds..
I think we should watch for the WM_SETTINGCHANGE in nsWindow and dispatch the NS_THEMECHANGE event (much like what my patch for bug 418454 does with WM_DWMCOMPOSITIONCHANGE). We should only dispatch the event if the flat menu setting changed (wParam should tell us this).
Comment on attachment 331965 [details] [diff] [review]
patch

I agree with Rob -- I think we should take that approach.  Kai, what do you think?
Attachment #331965 - Flags: superreview?(vladimir)
Attachment #331965 - Flags: superreview-
Attachment #331965 - Flags: review?(vladimir)
Attachment #331965 - Flags: review-

Comment 6

10 years ago
Yea, I agree.  I'm currently traveling and just checking my e-mails at night, but I'll see about making a new patch some time in the next few days.
QA Contact: themes → win32

Comment 7

7 years ago
Still present in 6.0 nightly. This should be pretty easy to fix. We addressed another bug similar to this with menus.
Assignee: kliu → nobody
Status: ASSIGNED → NEW

Comment 8

7 years ago
Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/13.0 Firefox/13.0a1 ID:20120220074932
Status: NEW → RESOLVED
Last Resolved: 11 years ago7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.