314843 - Change menu appearance to make it more OS/2 like

Status: RESOLVED FIXED

(Firefox :: Shell Integration, enhancement)

Description

[Peter Weilbacher]

As discussed in bug 314524 there is the possibility to get the menus of Firefox look more like native OS/2 program. It should pick up at least the colors of the active menuitem and the main menubar from the system. In a futher step one might want to change the hover effect to no produce a border etc.

Comment 1

[Peter Weilbacher]

Attachment: Preliminary patch to correct menu colors

This does the first step, to take system colors for menubar background and active menu item. For now, menu.css is without preprocessing directives which have to be added at a later stage.

[ Mikus, as you have unusual colors you would be a good person to test this. I have uploaded the wdgtos2.dll containing the C++ changes for the current Firefox trunk to <http://weilbacher.org/temp/wdgtos2_FF_trunk.zip>. The menu.css changes from the patch you would have to apply by hand... Any further suggestions are welcome. ]

Comment 2

[Peter Weilbacher]

Attachment: improved patch, includes widget+toolkit changes

OK, this patch now simulates the OS/2 look in the following ways:
- colors of the active menu items
- color of menu background
- height of menus
- space between menu items on main menubar
- no automatic opening of submenus by default (changed ui.submenuDelay to -1 in nsLookAndFeel.cpp)

I did not find a way to simulate:
- mark only menu items in submenus when they actually get clicked
- right keyboard selection of menus, they do not seem to get focused, so this is impossible
- The space of 3px to the side of selected menu items on main menubar that has SYSCLR_MENU color and not SYSCLR_MENUHILITEBGND
- does not redraw frame for system-wide color events (nor does it obey color drops from the palettes), like Alt+drop of color from color palette onto some menu (WM_SYSCOLORCHANGED would have to be processed). This only works after restarting the app (strange, for fonts it works already for the next new window?!). Looking at nsWindow on Windows this seems to require about 5 new methods that we don't have on OS/2 (yet).

I hope it's allowed to preprocess menu.css and toolbar.css so that we can continue to use winstripe on OS/2 without disturbing Windows. Not sure yet who to ask for sr...

Comment 3

[Peter Weilbacher]

Attachment: Screenshot before+after

Screenshots of Firefox before (left) and after (right) with the E editor as real OS/2 program for comparison in the middle.

Well, the menu height is not perfect but much better than before...

Comment 4

[Peter Weilbacher]

Attachment: Patch as used in 1.8 branch build

This is the patch that I used in my unofficial Firefox 1.5 release. It does not use OS/2 like colors as those depend on some other backend, trunk only changes from bug 243078.

I will upload an improved version of the earlier trunk patch hopefully tomorrow.

Comment 5

[Mike Kaply [:mkaply]]

I sincerely doubt this will be allowed in the code.

We could probably do this either:

1. OS/2 specific extension that overlaid the CSS.
2. separating a nativemenu.css/nativepopup.css. etc similar to what was done for Mac.

Comment 6

[Mike Connor [:mconnor]]

depending on how much else needs to change, you could always follow the lead of gnomestripe and selectively fork bits of winstripe...

Comment 7

[Mike Kaply [:mkaply]]

(In reply to comment #6)
> depending on how much else needs to change, you could always follow the lead of
> gnomestripe and selectively fork bits of winstripe...
> 

I'd been trying SO hard to make OS/2 follow the Windows path :)

Comment 8

[Peter Weilbacher]

And I agree that that's a good idea in principle to minimize the work we have to do for OS/2, but it just felt less and less like an OS/2 app with all that changes to make it look more natural on WinXP. And for now we only need our own versions of
   menu.css
   popup.css
   toolbar.css
   jar.mn
so it's not too bad. Let me see if I can get this to work.

Comment 9

[Peter Weilbacher]

Attachment: Backend change

OK, this backend change is OS/2 only anyway. Needed for trunk only so far.

Comment 10

[Peter Weilbacher]

Attachment: pmstripe

This patch creates an OS/2 "theme" I called pmstripe, and it works much like gnomestripe. I created the new files in the patch by diff'ing against an empty file. The appearance is not exactly like OS/2 native menus (the blue mark of selected menus in the main menubar is a bit wider) but it certainly looks much so than winstripe.

Comment 11

[Peter Weilbacher]

Attachment: Comparison with native menu

The new FF menu (lefthand side), a native OS/2 folder menu on the right

Comment 12

[Peter Weilbacher]

Mike, did you ever check in the backend change from attachment 207302 [details] [diff] [review]?

Any chance you are going to take a look at the "pmstripe" patch soon? Or should we find some other route of review/checkin for OS/2 stuff?

Comment 13

[Mike Kaply [:mkaply]]

Comment on attachment 207884 [details] [diff] [review]
pmstripe

This is OS/2 only so it doesn't need sr.

Sorry for the delay.

This is really nice work.

Comment 14

[Mike Kaply [:mkaply]]

Fixes checked in.

LEaving open just in case these don't magically go on the branch due to the mirroring.

Comment 15

[Peter Weilbacher]

Thanks for the checkins. 

I don't expect this to work on the branch(es) as it depends on some changes from bug 243078 (like menubarhovertext) that were never checked in for 1.8*. In this case automatic mirroring will hopefully fail (I don't see anything on 1.8 yet) and I think even manual porting to the branch will be difficult. Marking FIXED.

Comment 16

[Peter Weilbacher]

Comment on attachment 207884 [details] [diff] [review]
pmstripe

I want to bring the OS/2 "pmstripe" theme bits as developed in bug 314843 and bug 336997 onto the 1.8 branch. While it was originally stretched across several patches I am asking for branch approval for this one as a placeholder. What I will check in is the full pmstripe as fixed after the last checkin from bug 336997 as it has been tested on trunk. This patch is the only one that contains changes to non-OS/2 files.

OS/2 only, no risk, proven on trunk.

Comment 17

[Darin Fisher]

Comment on attachment 207884 [details] [diff] [review]
pmstripe

a=darin on behalf of drivers