Default icon should not be set for embedding applications

RESOLVED FIXED

Status

()

Core
XUL
--
major
RESOLVED FIXED
16 years ago
15 years ago

People

(Reporter: Marco Pesenti Gritti, Assigned: blizzard)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

16 years ago
The new gtk2 icon code override the icon set by galeon with the mozilla icons.
To reproduce just try TestGtkEmbed and see that it use the mozilla icon.
(Reporter)

Comment 1

16 years ago
Created attachment 95106 [details] [diff] [review]
proposed fix

The patch fix the problem by setting the default icon on Create, only when the
window type is not a child. So it will set it on the mozilla toplevel creation
but not on the gtkmozembed child creation.
I couldnt think to a cleaner fix ... hope it's ok.
(Reporter)

Comment 2

16 years ago
Just noticed that the patch doesnt fix the problem entirely, will try to
investigate more.
(Reporter)

Comment 3

16 years ago
Ok on some sites (www.linuxtoday.com, this bug page, www.slashdot.org), the
patch doesnt work because a toplevel (popup type) is created and immediately
destroyed ... Is this supposed to happen ?
Also I should have used mIsTopLevel instead of testing the type directly I guess.
(Reporter)

Comment 4

16 years ago
Created attachment 95122 [details] [diff] [review]
working patch

The patch leave the popup toplevel out of our test. Appear to work this time.
Attachment #95106 - Attachment is obsolete: true
(Reporter)

Comment 5

16 years ago
The problem is more serious than I thought. Overriding the icon cause critical
g_object_unref warnings and crashes when closing windows. (Changing severity)
Blocks: 92033
Severity: normal → major
Keywords: patch
(Assignee)

Comment 6

16 years ago
Maybe using the default icon is a bad idea.  Maybe we just need to set it for
toplevel windows every time.  That's reasonable.  This patch just sets the
default icon for the app once someone opens a toplevel window, which an app
might not want mozilla to do. :)
(Reporter)

Comment 7

16 years ago
I was assuming we had two cases
1 Embedding apps. A toplevel window is never created (through nsWindow) and we
dont want the default icon to be set.
2 Mozilla. A toplevel window is created and we want to set the default icon.

So, I'm sure I'm missing something, I cant get what is "which an app
might not want mozilla to do" referred to.

Anyway, setting the icon every time sounds like a good solution to me. Thanks
for looking into this.
(Assignee)

Comment 8

16 years ago
There's no way for the code to tell when Mozilla is embedded and when it isn't.
 And, frankly, I don't think that we should have to worry about it.  If we
create a toplevel window, we should be setting the icon, and we shouldn't use
the set_default at all.
(Reporter)

Comment 9

16 years ago
Created attachment 96714 [details] [diff] [review]
set the icon only on toplevel creation (eWindowType_dialog and eWindowType_toplevel(

Note that these days another icon is set outside the widget implementation
using SetIcon. 
So default.xpm is not visible (at least in the cases I verified). 
I wonder if we still need to set the icon in nsWindow at all ...
Attachment #95122 - Attachment is obsolete: true

Comment 10

15 years ago
Created attachment 103964 [details] [diff] [review]
The previous patch rediffed for current trunk

The previous patch didn't apply anymore.  I've updated it for current trunk.
(Assignee)

Comment 11

15 years ago
Created attachment 106445 [details] [diff] [review]
patch

Fixing icon handling.
Attachment #96714 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #103964 - Attachment is obsolete: true
(Assignee)

Comment 12

15 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.