gtk desktop icon should use the system default colormap, not the window's

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
16 years ago
12 years ago

People

(Reporter: Dan Dickerman, Assigned: Dan Dickerman)

Tracking

({fixed1.8})

Trunk
fixed1.8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

16 years ago
The problem shows-up on a display with multiple colormaps or multiple X Visuals,
where the colormap for the desktop does not match the colormap in use for the
browser window itself.  In my situation, for example, the desktop is a 8-plane
PseudoColor visual, and the window is either a private colormap 8-plane
PseudoColor or 24-plane TrueColor visual.  In either of these cases, the
colormap used for the window and that used for the icon should be different, but
(in milestone 0.9.6) Mozilla chooses the colormap for the icon based on that in
use by the window, causing the icon to appear with the wrong color choices on
the desktop -- usually either a vague outline of the icon, or otherwise random
color choices.

The code for nsWindow::SetIcon (widget/src/gtk/nsWindow.cpp) creates a pixmap
based on the current window's colormap, then passes this to gdk for use as the
desktop icon:
   w_pixmap = gdk_pixmap_create_from_xpm(mShell->window .... ) ...
   SetIcon(w_pixmap, w_mask)
     gdk_window_set_icon(...)

To force the use of the system default colormap for the desktop, the pixmap
could be created using the system default colormap, changing the call from
gdk_pixmap_create_from_xpm() to gdk_pixmap_colormap_create_from_xpm(NULL,
gdk_colormap_get_system()...).

Comment 1

16 years ago
adding myself & syd

Comment 2

15 years ago
Is this still a problem in a current build?

pi
(Assignee)

Comment 3

15 years ago
The behavior is unchanged in the Mozilla 1.0 build, and the code for
nsWindow::SetIcon() appears unchanged in widget/src/gtk/nsWindow.cpp in the
curent top-of-branch.  This is with gtk libraries v1.2.10; I haven't checked if
the behavior is the same in other gtk library revisions, but I would assume so.
(Assignee)

Comment 4

15 years ago
Created attachment 87273 [details]
showing the issue: screen grab of two icons with/without the correct cmap

Attachment shows two iconfied mozilla 1.0 browsers: the one on the left is what
I believe to be correct (with my suggested modifications), and the one on the
right is how it appears when the code from Moz 1.0 (and earlier) is used as
distributed.  The X server is configured to specify a default visual of 8-bit
PseudoColor, with the CDE desktop using this visual and taking up a good
portion of the default color map.  When Mozilla starts, gtk queries the server
for better color depth, and chooses the 24-bit TrueColor Visual for its own use
-- presumably the difference in color maps and colormap depths are causing the
differences seen here.
(Assignee)

Comment 5

15 years ago
Created attachment 87279 [details] [diff] [review]
suggested diffs as a patch

Fix as a patch, to afford easy fixing

Updated

15 years ago
Blocks: 123569
Dan, would you mind moving whitespace around so that:

1)  all the arguments to the function are lined up with each other
2)  The 80-char line length is exceeded as little as possible?

Do that and r=bzbarsky
Assignee: hyatt → dickermn
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: HP-UX → All
Hardware: HP → All
(Assignee)

Comment 7

15 years ago
Created attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

whitespace so adjusted: started the long function name on the next line to fit
the guidelines, which does look cleaner in the end.
Attachment #87279 - Attachment is obsolete: true
Comment on attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

r=bzbarsky; I have requested sr (per http://mozilla.org/hacking/reviewers.html)
Attachment #90086 - Flags: review+
Comment on attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

Chris, could you please sr this?  I mailed you way back when about it, and it
seems to have gotten lost in the bitbucket...
Attachment #90086 - Flags: superreview?(blizzard)
Comment on attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

Clearing old request
Attachment #90086 - Flags: superreview?(blizzard)
Comment on attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

Excuse me?  This patch has NOT been checked in.    It's still valid as far as I
can tell.  It still needs sr.  You're the obvious person to sr it, being most
familiar with this code.  If you don't have time to do the sr (and haven't
since a year ago), please just let me know and I'll look for someone else...
Attachment #90086 - Flags: superreview?(blizzard)
Comment on attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

OK.  I've spent over 3 years here trying to get review for a contributed
patch...  I don't think I care whether it gets review from a GTK person
anymore.  Shaver, could you sr, please?
Attachment #90086 - Flags: superreview?(blizzard) → superreview?(shaver)
Comment on attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

sr=shaver!
Attachment #90086 - Flags: superreview?(shaver) → superreview+
blizzard's clearly not module owner -- I'd have fired him already, but he quit
long ago.  He has not appointed a successor.  Does anyone have suggestions for a
new owner?

/be
Fixed on trunk.  Dan, our apologies that this took so long to deal with.  I
promise you that most modules in the Mozilla tree are in much much better shape
as far as having someone actually responsible for them.  :(
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Comment on attachment 90086 [details] [diff] [review]
revised whitespace version of previous patch

I think we should take this on the 1.8b5 branch... It's very low-risk and helps
a good bit with not looking really weird in the cases when colormaps actually
matter.
Attachment #90086 - Flags: approval1.8b5?
(Assignee)

Comment 17

12 years ago
Hey no worries on the timescale: since this bug was filed, our first and second child were born (June 2002 
and June 2004), so we've been a little too busy to fuss with the finer points of colormap issues.

Updated

12 years ago
Attachment #90086 - Flags: approval1.8b5? → approval1.8b5+
Fixed on 1.8 branch.
Keywords: fixed1.8
You need to log in before you can comment on or make changes to this bug.