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)

x86
Linux
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.2final

People

(Reporter: frederik.reiss, Assigned: bryner)

References

Details

(Keywords: classic, crash)

Attachments

(1 file, 1 obsolete file)

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
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.
URL: n/a
Keywords: crash
gtk theme bugs -> me
Assignee: ben → bryner
Status: UNCONFIRMED → NEW
Status: NEW → ASSIGNED
Ever confirmed: true
Target Milestone: --- → mozilla1.2final
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?
Hi,

I'm using version 0.7. This version comes with Debian 3 (woody)
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.
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?
*** Bug 175759 has been marked as a duplicate of this bug. ***
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.
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.
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?
I could not reproduce the problem in 0.8.  What theme were you using with the
Xenophilia engine?
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?
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.
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.
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?
Sorry for the spam, but is bug 175755 maybe some kind of this bug ?
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.
Attached patch simple blacklist implementation (obsolete) — Splinter Review
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.
removed an unused member variable
Attachment #104131 - Attachment is obsolete: true
bryner:
What about using "accept"/"reject" regex patterns set via prefs instead of a
hardcoded list ?
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.
I also get this bug with xenophilia-engine version 0.8 (Debian package
gtk-engines-xenophilia 0.8-1) with theme "Xeno Gradient".
ah, yes, the above is with phoenix 0.4
When i install the gtk-engines-xenophilia 0.8-1 Debian Package Mozilla doese'nt
crash any more with xenophilia GTK-Themes.
*** Bug 177465 has been marked as a duplicate of this bug. ***
Fixed for me in Phoenix 0.4 by upgrading to Xenophilia 0.8 (compiled from
vanilla source tarball).
*** Bug 177601 has been marked as a duplicate of this bug. ***
Comment on attachment 104132 [details] [diff] [review]
small fix to previous patch

r/sr=blizzard
Attachment #104132 - Flags: superreview+
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 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+
checked into the trunk and MOZILLA_1_2_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
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
   };
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?
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
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.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: