Closed Bug 314843 Opened 14 years ago Closed 14 years ago

Change menu appearance to make it more OS/2 like

Categories

(Firefox :: Shell Integration, enhancement)

x86
OS/2
enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mozilla, Assigned: mozilla)

Details

(Keywords: fixed1.8.1)

Attachments

(4 files, 3 obsolete files)

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.
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. ]
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
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...
Attachment #201695 - Attachment is obsolete: true
Attachment #202934 - Flags: review?(mozilla)
Attached image Screenshot before+after (obsolete) —
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...
Attachment #202934 - Flags: superreview?(mconnor)
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.
Attachment #202934 - Attachment is obsolete: true
Attachment #202934 - Flags: superreview?(mconnor)
Attachment #202934 - Flags: review?(mozilla)
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.
depending on how much else needs to change, you could always follow the lead of gnomestripe and selectively fork bits of winstripe...
(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 :)
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.
Attached patch Backend changeSplinter Review
OK, this backend change is OS/2 only anyway. Needed for trunk only so far.
Attachment #207302 - Flags: superreview?(mozilla)
Attachment #207302 - Flags: review?(mozilla)
Attachment #207302 - Flags: superreview?(mozilla)
Attachment #207302 - Flags: superreview+
Attachment #207302 - Flags: review?(mozilla)
Attachment #207302 - Flags: review+
Attached patch pmstripeSplinter Review
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.
Attachment #202935 - Attachment is obsolete: true
Attachment #207884 - Flags: superreview?
Attachment #207884 - Flags: review?(mozilla)
Attachment #207884 - Flags: superreview? → superreview?(mconnor)
The new FF menu (lefthand side), a native OS/2 folder menu on the right
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 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.
Attachment #207884 - Flags: superreview?(mconnor)
Attachment #207884 - Flags: review?(mozilla)
Attachment #207884 - Flags: review+
Fixes checked in.

LEaving open just in case these don't magically go on the branch due to the mirroring.
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.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
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.
Attachment #207884 - Flags: approval1.8.1?
Comment on attachment 207884 [details] [diff] [review]
pmstripe

a=darin on behalf of drivers
Attachment #207884 - Flags: approval1.8.1? → approval1.8.1+
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.