Closed
Bug 175306
Opened 22 years ago
Closed 22 years ago
Blacklist GTK themes [Xenophilia <0.8 engine] that crash mozilla 1.2b classic theme
Categories
(Core Graveyard :: Skinability, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.2final
People
(Reporter: frederik.reiss, Assigned: bryner)
References
Details
(Keywords: classic, crash)
Attachments
(1 file, 1 obsolete file)
2.49 KB,
patch
|
brendan
:
review+
blizzard
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2b) Gecko/20021016 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2b) Gecko/20021016 Mozilla (classic theme) crashes with some GTK Themes (e.g. NeXTStep, AquaXen). It looks like this happens with themes that have both arrows for the scrollbar at the bottom or the top. With the modern Theme is all ok. Reproducible: Always Steps to Reproduce: 1. edit your .gtkrc to use the NeXTStep theme. 2. start mozilla Actual Results: mozilla crashes at startup ~1 sec after the mozilla window appears Expected Results: Mozilla should work with all GTK themes
Reporter | ||
Comment 1•22 years ago
|
||
Oh, i Forgot: when is start mozilla form a console GTK Prints 2 error messages when mozilla crshes: drd@bigbad:~$ /opt/mozilla/mozilla Gdk-CRITICAL **: file gdkwindow.c: line 1440 (gdk_window_get_colormap): assertion `window_private->window_type != GDK_WINDOW_PIXMAP' failed. Gdk-CRITICAL **: file gdkcolor.c: line 498 (gdk_color_white): assertion `colormap != NULL' failed.
Updated•22 years ago
|
Assignee | ||
Comment 2•22 years ago
|
||
gtk theme bugs -> me
Assignee: ben → bryner
Status: UNCONFIRMED → NEW
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Ever confirmed: true
Target Milestone: --- → mozilla1.2final
Assignee | ||
Comment 3•22 years ago
|
||
I was unable to reproduce this using the Xenophilia 0.8 engine with the NeXTStep theme. Which version of the Xenophilia engine are you using?
Reporter | ||
Comment 4•22 years ago
|
||
Hi, I'm using version 0.7. This version comes with Debian 3 (woody)
Assignee | ||
Comment 5•22 years ago
|
||
With Xenophilia 0.7, there seems to be a bug that occurs if you pass a pixmap instead of a window to gtk_paint_box(). It ends up calling gdk_pixmap_create_from_xpm, passing the pixmap for the GdkWindow paramter, which has to actually be a GdkWindow. That function returns early, then Xenophilia later crashes. This code seems to have been rewritten for Xenophilia 0.8. Specifically, it no longer uses GdkPixmaps for anything.
Comment 6•22 years ago
|
||
Is there any way to catch segfaults inside theme code in the same way as we catch other errors (ie disable the particular widget type in question)? Perhaps by setting up a signal handler for SEGV?
Assignee | ||
Comment 7•22 years ago
|
||
*** Bug 175759 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 8•22 years ago
|
||
I'm not sure a SEGV handler is workable here. We'd basically want to jump several stack frames and resume execution in Mozilla's code after the call to the theme drawing function. I can't think of any way to do that (at least, not without some low-level hacking that I really don't want to do). A slightly safer and more conservative approach is to blacklist problematic theme engines. "xeno" would be going on the list. Unfortunately, there's no way that I can see to obtain the version of a theme, which is a shame because the latest version of Xenophilia doesn't actually have problems. I could hack it to not blacklist "xeno" if the library exports some particular symbol that is only in Xenophilia 0.8 and higher (it's obviously fragile, as would be the reverse approach of requiring a certain symbol to be there to blacklist). Another idea is to make it known that an engine could export a particular symbol (i.e. "mozilla_theme_support") to declare itself safe for Mozilla. Not sure if that would catch on with theme engine authors or not.
I run xenophilia-0.8 and my bug just got marked a dup of this one so I would stress that if it is really a dup then 0.8 does NOT fix your segfault.
Reporter | ||
Comment 10•22 years ago
|
||
I have tried this with a phoenix nightly (2002-10-21-08), and i get the same error as in mozilla, so i think Bug 175759 is realy a dupe of this bug.
Comment 11•22 years ago
|
||
I really don't know what I'm talking about here, so I should probably just shut up, but... would setjmp/longjmp help with the "jump up through several stack frames" problem?
Assignee | ||
Comment 12•22 years ago
|
||
I could not reproduce the problem in 0.8. What theme were you using with the Xenophilia engine?
Assignee | ||
Comment 13•22 years ago
|
||
setjmp may be useful here, yeah. blizzard, roc, what do you guys think? Should we try to catch crashes in the theme engines and disable the widgets? Or should we blacklist/whitelist theme engines?
Comment 14•22 years ago
|
||
I would _much_ rather add blacklisting than catching segvs and trying to recover. There might be all kinds of internal state we're losing down in X or in gdk/gtk if we do that.
Reporter | ||
Comment 15•22 years ago
|
||
I really don't know what I'm talking about too, but i think you should investigate this bug and not just blacklist the engines, because the engine work's with other apps, only mozilla & co. crash -> the problem is somewere in mozilla, and NOT the theme engine itself.
Comment 16•22 years ago
|
||
Actually the bug has already been investigated and it *is* a problem in the theme. The theme assumes that it's writing to an actual Window, which isn't a safe assumption for a theme to make. Mozilla asks it to write to a Pixmap (basically an offscreen drawing location) and the theme crashes on that. Some other themes also assume that they're working on Window objects but give recoverable errors, rather than just crashing, and Mozilla knows how to handle that. I don't think there's any way (?) to do what Mozilla needs to do and yet be able to pass a real Window to the theme. Is there?
Reporter | ||
Comment 17•22 years ago
|
||
Sorry for the spam, but is bug 175755 maybe some kind of this bug ?
Comment 18•22 years ago
|
||
Well, saying "it's a problem with the theme" is kind of pushing the envelope. We're certainly doing things with the theme engines that other people have not done in the past and we're requiring the engines to make assumptions about where they are drawing that have not been made in the past. So I wouldn't call it a bug in either mozilla or the theme engine, really, more like an incompatibility. That's why I think that blacklisting is OK.
There is no way to catch a theme engine crash and recover. The best we could do is catch a theme engine crash, disable native themes, and then restart Mozilla. I think a theme blacklist is the way to go.
Assignee | ||
Comment 20•22 years ago
|
||
This uses a hardcoded blacklist of theme engines, currently containing only the "xeno" engine. If this theme engine is being used, it disables all widget types.
Assignee | ||
Comment 21•22 years ago
|
||
removed an unused member variable
Attachment #104131 -
Attachment is obsolete: true
Comment 22•22 years ago
|
||
bryner: What about using "accept"/"reject" regex patterns set via prefs instead of a hardcoded list ?
Assignee | ||
Comment 23•22 years ago
|
||
We don't need regexp patterns. I fail to see how any kind of pattern matching would be useful here. Making the list configurable at runtime is a possibility, but I'd want to do this in a way that wouldn't slow startup time.
Comment 24•22 years ago
|
||
I also get this bug with xenophilia-engine version 0.8 (Debian package gtk-engines-xenophilia 0.8-1) with theme "Xeno Gradient".
Comment 25•22 years ago
|
||
ah, yes, the above is with phoenix 0.4
Reporter | ||
Comment 26•22 years ago
|
||
When i install the gtk-engines-xenophilia 0.8-1 Debian Package Mozilla doese'nt crash any more with xenophilia GTK-Themes.
Comment 27•22 years ago
|
||
*** Bug 177465 has been marked as a duplicate of this bug. ***
Comment 28•22 years ago
|
||
Fixed for me in Phoenix 0.4 by upgrading to Xenophilia 0.8 (compiled from vanilla source tarball).
Comment 29•22 years ago
|
||
*** Bug 177601 has been marked as a duplicate of this bug. ***
Comment 30•22 years ago
|
||
Comment on attachment 104132 [details] [diff] [review] small fix to previous patch r/sr=blizzard
Attachment #104132 -
Flags: superreview+
Comment 31•22 years ago
|
||
Comment on attachment 104132 [details] [diff] [review] small fix to previous patch struct _GtkThemeEnginePrivate may be following some local convention, but it violates the ISO C implementation namespace to start the struct tag's identifier with _, and it is not necessary to avoid any user-namespace collision or pollution. Use memset here: + for (unsigned int j = 0; j < sizeof(mDisabledWidgetTypes); ++j) + mDisabledWidgetTypes[j] = 0xff; Fix these, and sr=brendan@mozilla.org (blizzard's sr= should be an r=, I think). /be
Attachment #104132 -
Flags: review+
Comment 32•22 years ago
|
||
Comment on attachment 104132 [details] [diff] [review] small fix to previous patch a=asa for checkin to 1.2 branch (on behalf of drivers)
Attachment #104132 -
Flags: approval+
Assignee | ||
Comment 33•22 years ago
|
||
checked into the trunk and MOZILLA_1_2_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 34•22 years ago
|
||
Just a thought. How about putting a bug numbers for blacklisted theme engines so the exact reasons for blacklisting can be fould easier? For example const char* nsNativeThemeGTK::sDisabledEngines[] = { - "xeno", // xenophilia + "xeno", // xenophilia (bug 175306) nsnull };
Comment 35•22 years ago
|
||
Will a way be provided to disable blacklisting (short of patching and recompiling the source, or changing the name of the gtk engine) for those of us for whom xeno happens to work?
Comment 36•22 years ago
|
||
Brian Ryner wrote: > We don't need regexp patterns. I fail to see how any kind of pattern matching > would be useful here. My idea was to add a simple prefs to define accept/reject patterns for themes like the X font banning code does (so you can define a list of wanted themes and/or a list of themes which should be banned). Startup time won't be hurt by fetching one single prefs and two regex matches (unless you start to mangle input strings via lots of |sprintf()| per match - which caused the Ts-regression for the font banning code). > Making the list configurable at runtime is a possibility, > but I'd want to do this in a way that wouldn't slow startup time. Is there a bug yet to make the list configurable via prefs ?
Keywords: classic
Summary: mozilla 1.2b (classic theme) crashes with some GTK themes → Blacklist GTK themes [Xenophilia <0.8 engine] that crash mozilla 1.2b classic theme
Assignee | ||
Comment 37•22 years ago
|
||
I wasn't planning to invest a lot of effort into the theme engine blacklisting, in the hope that it will be a temporary measure. I definitely have more important bugs on my plate right now. If someone else wants to do the work I will review it.
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•