280791 - System menu icon missing on OS/2

Status: RESOLVED WORKSFORME

(Firefox Build System :: General, defect)

Description

[Peter Weilbacher]

The missing menu icon is because the file chrome\icons\default\main-window.ico
is left out from the Firefox package. This was true for the 1.0 releases and is
still true if I produce a package with make -C xpinstall/packager/ from the
current trunk.

It's probably easy to fix but I didn't have time to investigate yet, so this is
just a placeholder to remind myself.

Comment 1

[Mike Kaply [:mkaply]]

strange. I don't see any main-window icon references in toolkit or browser at all.

Comment 2

[Peter Weilbacher]

For thunderbird it works because other-licenses/branding/thunderbird/Makefile.in
does a

cp $(srcdir)/thunderbird-os2.ico  $(DEPTH)/mail/app/icons/os2/messengerWindow.ico

while other-licenses/branding/firefox/Makefile.in has nothing like this. 

Don't know why it works, but if I copy a file there as main-window.ico I get the
icon in the system menu. Does it work on other platforms? If so there was
probably some mechanism implemented for Firefox (but not Thunderbird) that was
not picked up on OS/2.

Comment 3

[Peter Weilbacher]

When I compile my own Firefox build on Linux (GTK1) I see the same problem, the
directory chrome/icons is not there. However, the official Firefox 1.0 installer
(GTK2) contains chrome/icons/default/default.xpm and then the system icon is
displayed if I used e.g. sawfish as window manager.

I guess this means we have to add something like this line
http://lxr.mozilla.org/seamonkey/source/browser/app/Makefile.in#259
for OS/2.

Comment 4

[Mike Kaply [:mkaply]]

What's really strange is that my Windows build doesn't have chrome/icons but has
the icon on the window - probably a platform difference.

Comment 5

[Mike Kaply [:mkaply]]

I noticed something very interesting on my mom's machine this weekend - Win 98
does not have the system menu icon.

I'm pretty sure it does on Firefox.

So that means that whatever is making the system icon work on WinXP is probably
something different than the Mozilla way.

So we should do a patch for this and check it in.

Comment 6

[Peter Weilbacher]

Attachment: copy icon from branding

This creates a new directory icons\default in chrome and then copies the icon
file from branding. I don't know what the install:: part does for the GTK2 case
but on OS/2 the libs:: case seems to be enough. (I didn't make a full compile,
but a partial compile of mozilla\browser worked.)

Comment 7

[Andy Willis (abwillis)]

Is it possible this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=282549
affects this?

Comment 8

[Mike Kaply [:mkaply]]

Nope. Two different things.

Comment 9

[Mike Kaply [:mkaply]]

This patch works but I wish the nsinstall worked properly and created the nested
directories rather than having to explicitly create the directories and copying it.

I'm still thinking.

Comment 10

[Andy Willis (abwillis)]

I think the problem is in the browsers\app\makefile.in
147 ifeq ($(OS_ARCH),OS2)
148 RESFILE=splashos2.res
149 RCFLAGS += -DMOZ_PHOENIX
150 ifdef BUILD_STATIC_LIBS
151 EXE_DEF_FILE = browser.def
152 RCFLAGS += -DMOZ_STATIC_BUILD -i $(DIST)/include/widget
153 endif
154 ifdef DEBUG
155 RCFLAGS += -DDEBUG
156 endif
157 RCFLAGS += -DFIREFOX_ICO=\"$(DIST)/branding/firefox.ico\"
-DDOCUMENT_ICO=\"$(DIST)/branding/document.ico\"
158 endif
159 

I think it should be firefox-os2.ico and document-os2.ico.
Andy

Comment 11

[Peter Weilbacher]

No, that part works, otherwise firefox.exe wouldn't get an icon. (Have a look
into obj/dist/branding and you can see firefox.ico and document.ico. They get
copied there at some points.) This is about something else, and if you want to
get the sysmenu icon you can use the attached patch. But it's a ugly hack...

Comment 12

[Peter Weilbacher]

This was now obsoleted, at least it now works in a build I just did from the
trunk. I think by one of Rich's patches but I can't remember which one otherwise
I would dupe it to the respective bug.

Comment 13

[Mike Kaply [:mkaply]]

This is working for me now on all builds.