Closed Bug 284555 Opened 16 years ago Closed 15 years ago

Menu text color remains -moz-menuhovertext when popup a dialog

Categories

(Core Graveyard :: GFX, defect)

Sun
Solaris
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

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

References

Details

(Keywords: fixed1.8.1, regression)

Attachments

(2 files, 1 obsolete file)

I'm using GTK2 20050217 nightly build.

Try Edit->Preferences

The color of "Edit" in top menu remains -moz-menuhovertext.
It should be color MenuText.
Attached image Screenshot
With 20050302 nightly build
Preferences dialog is changing.
But this bug is still exsit with File->Page Setup
A regression of bug 281568
Depends on: 281568
Assignee: firefox → aaronleventhal
Ginn, can you do me a favor and take this one? The bug doesn't occur on Windows
and it will take me a while to get a Linux machine going again. Sorry for the
regression. Also, Kyle said you enjoy fixing the bugs I cause :)
Assignee: aaronleventhal → ginn.chen
Component: Menus → GFX
Product: Firefox → Core
Backing out this line solved this bug, but I don't know if it will break bug
281568 again.
Attachment #177838 - Flags: review?(aaronleventhal)
Aaron, I just tested my patch on Windows with MSAA.
Here's the result.

1) Before fixing of bug 281568, the event sequence is
MENUSTART
MENUPOPUPSTART
Click 'Cancel'
MENUPOPUPEND
MENUEND

2) Nightly trunk, the event sequence is
MENUSTART
MENUPOPUPSTART
MENUPOPUPEND
MENUEND
Click 'Cancel'

3) Use my patch to back out 1 line, the event sequence is
MENUSTART
MENUPOPUPSTART
MENUPOPUPEND
Click 'Cancel'
MENUEND

Do you think the sequence 3) is right?
Attachment #177838 - Attachment is obsolete: true
Attachment #177838 - Flags: review?(aaronleventhal)
Attached patch patchSplinter Review
Use ToggleMenuActiveState() instead of just SetActive(PR_TRUE), so that the
menu will be deactivated correctly.
The 'if (mIsActive)' is needless, put it here just in case.
Attachment #190809 - Flags: review?(aaronleventhal)
(In reply to comment #6)
> Do you think the sequence 3) is right?
The sequence 3) is wrong.
So I made another patch.

I've tested on Linux and Windows. It fixes this bug and doesn't take bug 281568 
back.

Aaron, please review it.

Status: NEW → ASSIGNED
Comment on attachment 190809 [details] [diff] [review]
patch

It looks okay, because ToggleMenuActiveState() just does the same thing as
SetActive() but also closes any open submenu.
Attachment #190809 - Flags: review?(aaronleventhal) → review+
BTW, I tested with a screen reader on Windows and going into a dialog from a
menu still reads correctly, so this fix definitely doesn't break bug 281568.
Comment on attachment 190809 [details] [diff] [review]
patch

Thank you, aaron.
Neil, since you superreviewed patch of bug 281568, please sr this again.
Thanks.
Attachment #190809 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #190809 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview?(roc)
Attachment #190809 - Flags: superreview?(roc) → superreview+
Checking in xul/base/src/nsMenuBarFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuBarFrame.cpp,v  <--  nsMenuBarFrame.cpp
new revision: 1.135; previous revision: 1.134
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Blocks: 290085
Blocks: 312298
QA Contact: bugzilla → general
Comment on attachment 190809 [details] [diff] [review]
patch

It still happens when pops up "Confirm close" dialog on MOZILLA_1_8_BRANCH.
Attachment #190809 - Flags: approval-branch-1.8.1?(roc)
Attachment #190809 - Flags: approval-branch-1.8.1?(roc) → approval-branch-1.8.1+
Checking in xul/base/src/nsMenuBarFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuBarFrame.cpp,v  <--  nsMenuBarFrame.cpp
new revision: 1.131.14.3; previous revision: 1.131.14.2
done
Keywords: fixed1.8.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.