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

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect)

Description

[Ginn Chen]

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.

Comment 1

[Ginn Chen]

Attachment: Screenshot

Comment 2

[Ginn Chen]

With 20050302 nightly build
Preferences dialog is changing.
But this bug is still exsit with File->Page Setup

Comment 3

[Ginn Chen]

A regression of bug 281568

Comment 4

[Aaron Leventhal]

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 :)

Comment 5

[Ginn Chen]

Attachment: patch (back out 1 line of bug 281568)

Backing out this line solved this bug, but I don't know if it will break bug
281568 again.

Comment 6

[Ginn Chen]

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?

Comment 7

[Ginn Chen]

Attachment: patch

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.

Comment 8

[Ginn Chen]

(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.

Comment 9

[Aaron Leventhal]

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.

Comment 10

[Aaron Leventhal]

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 11

[Ginn Chen]

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

Thank you, aaron.
Neil, since you superreviewed patch of bug 281568, please sr this again.
Thanks.

Comment 12

[Ginn Chen]

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

Comment 13

[Ginn Chen]

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

It still happens when pops up "Confirm close" dialog on MOZILLA_1_8_BRANCH.

Comment 14

[Ginn Chen]

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