368864 - Menu selection is invisible in SeaMonkey 1.1 on Warp 3

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[lvm]

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 3; en-US; rv:1.8.0.9) Gecko/20061221 SeaMonkey/1.0.7
Build Identifier: Mozilla/5.0 (OS/2; U; Warp 3; en-US; rv:1.8.0.9) Gecko/20061221 SeaMonkey/1.0.7

Drop-down menu selection is invisible in the classic (but not in the modern) theme. Replacing classic.jar for the one from the previous (1.0.7) version wich didn't have this problem didn't help. 1.1 for windows works fine. Menu background color is the default lightgray (0xcccccc). Problems setting -moz-menuhover color to the system default?


Reproducible: Always

Steps to Reproduce:
1. open any drop-down menu whith a keyboard or mouse
2. try moving selection with up/down keys or with a mouse

Actual Results:  
nothing happens although pressing enter shows that menu selection moves but I cannot see it

Expected Results:  
menu selection shoould be highlighted with the system menu selection color

Comment 1

[lvm]

Sorry, I've already rolled back to 1.0.7 but forgot to paste correct user-agent - this bug applies only to 1.1, 1.0.7 works fine

Comment 2

[Peter Weilbacher]

For me it works. In fact I thought that it is one of the new things with 1.1 that it now actually for the first time respects the system colors for these things. Can you attach a screenshot of what a menu of a real OS/2-PM app and menus of SM 1.0.7 and SM 1.1 look like for you?

Comment 3

[Peter Weilbacher]

Oh, you are still using Warp 3? That might make a difference in how the system colors are queried, although I don't know what the difference should be programmatically. (I'm kind of surprised that it runs for you at all as Warp 4 has been a base requirement for quite some time.)

Comment 4

[lvm]

Attachment: screenshots for Peter Weilbacher

Come to think of it, the current behavior is completely logical - and completely wrong :) My color setup which as far as menus are concerned matches the default system setup, uses the same menu highlight background color as menu background color, highlight bar is visible only because of 3D relief (plainvanillamenu.png in the attached .zip). Seamonkey replaces system drop-down menus with the abomination called skins which does not support OS/2 3D highlight. 1.0.7 and earlier versions used 'Selection Background' color for menu highlight (green in my setup - seamonkey1.0.7.png), now you put it right, menu highlight uses the true menu highlight color - and cannot be seen as there is no 3D relief to separate if from the background (seamonkey1.1.png - don't know how it will help you - it is one of those 'you can see that you can see nothing' things, but I followed the same steps as when making the 1.0.7 screenshot). Somehow I don't feel you did the right thing. Please unfix it - at least until seamonkey theme manager learns to draw 3D menus.

Comment 5

[Peter Weilbacher]

Thanks, now at least I understand what's going on. It really sucks...

I could change the theme so that it draws the 3D relief, that's no problem. But then it wouldn't look right on Warp 4 and higher. And in the theme files I have no way of querying the system.

Perhaps the best solution for you is to edit the theme file yourself. I think it's the file skin/classic/global/menu.css in chrome/classic.jar that needs to be changed. If you know a bit of CSS you can play with that yourself:
- cd chrome
- copy classic.jar classic.jar.backup
- unzip classic.jar skin/classic/global/menu.css 
- edit skin/classic/global/menu.css 
- zip -u0D classic.jar skin/classic/global/menu.css
If not I will give it a try when I have a few minutes -- which unfortunately is very rare. :-(

Comment 6

[Peter Weilbacher]

Well, never had time to look into this further. But as someone on the newsgroup now asked about the same problem, remembered it. Instead of editing the stuff in the .jar files, a better idea would be to override some menu properties using userChrome.css (create it in <Profile>/chrome if it doesn't exist yet). 

Something like

   menubar > menu[_moz-menuactive="true"][open="true"],
   menupopup > menu[_moz-menuactive="true"],
   menupopup > menuitem[_moz-menuactive="true"],
   popup > menu[_moz-menuactive="true"],
   popup > menuitem[_moz-menuactive="true"] {
     background: blue !important;
     color: white !important;
  }

should at least give you something visible. To make it look right, one would need to play with CSS borders and their colors.

Comment 7

[Peter Weilbacher]

Attachment: Test for equal colors (checked in, trunk and 1.8 branch)

OK, this is the idea of Rudi Ihle, compare the background menu color and the highlight. If they are equal (as should happen on Warp3) then deliver an artificial dark gray background and white text.

I cannot test if it actually works, though. I don't have Warp3 and on Warp4 I never get completely equal colors. So my idea would be to get this into the branch nightlies to get something to test for the Warp3 users.

Comment 8

[Mike Kaply [:mkaply]]

Comment on attachment 293023 [details] [diff] [review]
Test for equal colors (checked in, trunk and 1.8 branch)

Looks reasonable to me.

Comment 9

[lvm]

Patching menu.css may be not elegant but it allows freedom of choice, suggested hardcode will make it impossible. There is no accounting for tastes, for me white on gray combination looks quite disgusting, I'd rather keep restoring menu.css with each new version. Please don't fix it this way.

Versions before 1.1 used selection background color for menu bar highlight and everyone seemed reasonably happy - maybe you should restore this behavior in case colors of menu background and highlight match?

Comment 10

[Peter Weilbacher]

lvm: no, the suggested patch would just set the defaults, so you can see something. You can still override those colors by adapting userChrome.css to your liking. (The same way you can override basically anything of the displayed user interface.) I will check in the patch tonight, I hope you can then try the nightly build after that to confirm that you can see something, and that the CSS override works.

And no, I won't change it back to look good on Warp3 but out of place on Warp4 and eCS, which are the supported platforms. That old look bothered me a lot.

Comment 11

[Peter Weilbacher]

OK, checked the patch into branch and trunk now.

So as soon as nightlies with a date of 15 Dec 2007 or later appear in

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/contrib/latest-mozilla1.8/
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/contrib/latest-mozilla1.8/
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/contrib/latest-mozilla1.8/

this patch should be in there, resulting in white on dark gray selected menu items on Warp3 by default.

Comment 12

[lvm]

Sorry to say this bug is not fixed, menu selection is still invisible on Warp3 in Mozilla/5.0 (OS/2; U; Warp 3; en-US; rv:1.8.1.12pre) Gecko/20071221 SeaMonkey/1.1.8pre. Setting up colors in userChrome.css is indeed neater than editing classic.jar, thanks for the tip.

Comment 13

[Peter Weilbacher]

Thanks for testing. So, you are saying that nothing changed at all? That'S weird. Perhaps I should have left some debugging output in the builds to find why that is...

Fine if you are happy with userChrome.css but now that I started this in the code I also want to go all the way and really make it at least visible -> reopen.

Comment 14

[lvm]

I wouldn't say that nothing has changed - I didn't patch each new classic.jar, I did it once quite a while ago and used it with all seamonkeys after this problem started. Now I tried the current classic.jar and noticed that scrollbars became different, menu-right-arrow has changed (IMHO for worse) and, much more annoyingly, menu line spacing has increased by several pixels, so now a couple less lines of bookmarks fits to a screen in the same 10px font - the last I definitely would like to be fixed, but that's outside the scope of the current bug - here indeed is no change, menu selection without userChrome.css or classic.jar tweaks is still invisible.

Comment 15

[Peter Weilbacher]

Well, everything looks more like Warp 4 now (to benefit the >800 users that are running Warp 4 against the <10 users that are still running Warp 3), I'm not surprised that you don't look right on Warp 3. So unless you want to add many more rules to your userChrome.css I would say that the classic.jar method is best for you.

In any case, I found that the logic of the patch was severely flawed (I was comparing color indices instead of the colors themselves), so I created a new testcase and uploaded this to <http://temp.weilbacher.org/wdgtos2_bug368864.zip>. The contained DLL should be unpacked into an installation of SeaMonkey 1.1.7 so that it overwrites the DLL that is present (backup?!). In case this still doesn't work, I would like to see the output of a run like this
   seamonkey > sm_color.log 2>&1

Comment 16

[lvm]

still invisible, here is the log

MENU=13421772
MENUTEXT=0
MENUDISABLEDTEXT=8421504
MENUTEXT(button(hover)text)=0
MENUHILITEBGND=0xcccccc, MENUHILITE=0
MENUHILITEBGND=0xcccccc, MENUHILITE=0

Comment 17

[Peter Weilbacher]

Oh my god, I was really blind before and compared the wrong properties!

I have put up another DLL for testing to <http://temp.weilbacher.org/wdgtos2_bug368864_v3.zip> which hopefully compare the correct ones...

Comment 18

[Peter Weilbacher]

Attachment: patch with correct properties (checked in, trunk and 1.8 branch)

This is the patch (1.8 branch based) to go with the test DLL.

Comment 19

[lvm]

you did it

Comment 20

[Peter Weilbacher]

OK, great. But not FIXED yet, I still need to check the patch into the trees...

Comment 21

[Peter Weilbacher]

Comment on attachment 294573 [details] [diff] [review]
patch with correct properties (checked in, trunk and 1.8 branch)

Mike, does this look better to you, too?

Comment 22

[Mike Kaply [:mkaply]]

Comment on attachment 294573 [details] [diff] [review]
patch with correct properties (checked in, trunk and 1.8 branch)

Yeah, that looks better.

I assumed the reason the other one worked is because the #defines were the same on Warp3, but in hindsite of course that wouldn't work.

Comment 23

[Peter Weilbacher]

Comment on attachment 293023 [details] [diff] [review]
Test for equal colors (checked in, trunk and 1.8 branch)

This first patch was checked into trunk on 2007-12-14 12:30 and 1.8 branch on 2007-12-14 12:33.

Comment 24

[Peter Weilbacher]

Comment on attachment 294573 [details] [diff] [review]
patch with correct properties (checked in, trunk and 1.8 branch)

And this follow-up patch was checked in just now, (2007-12-28 15:34).

Comment 25

[Peter Weilbacher]

So now it is fixed for real. :-)

Comment 26

[Peter Weilbacher]

And lvm has verified that it works.