Last Comment Bug 389801 - Firefox crashes with some GTK+ themes whose gtkrc contains GtkOptionMenu::indicator_size and GtkOptionMenu::indicator_spacing
: Firefox crashes with some GTK+ themes whose gtkrc contains GtkOptionMenu::ind...
Status: RESOLVED FIXED
: crash, fixed1.8.0.14, fixed1.8.1.8
Product: Core Graveyard
Classification: Graveyard
Component: GFX: Gtk (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: mozilla1.9beta1
Assigned To: Frederic Crozat
:
Mentors:
http://bonsai.mozilla.org/cvsblame.cg...
: 394876 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-27 00:02 PDT by Hwasung Kim
Modified: 2010-09-18 09:45 PDT (History)
7 users (show)
reed: blocking1.9?
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
like this? (604 bytes, patch)
2007-07-29 15:53 PDT, timeless
no flags Details | Diff | Splinter Review
more like this? (660 bytes, patch)
2007-07-29 15:54 PDT, timeless
chpe: review-
Details | Diff | Splinter Review
details... (1.07 KB, patch)
2007-07-29 16:08 PDT, timeless
chpe: review-
Details | Diff | Splinter Review
ok, patching whlie sleepy is bad? (1.07 KB, patch)
2007-07-29 23:19 PDT, timeless
chpe: review+
roc: superreview+
roc: approval1.9+
Details | Diff | Splinter Review
better patch (484 bytes, patch)
2007-09-06 00:44 PDT, Frederic Crozat
chpe: review+
roc: superreview+
dveditz: approval1.8.1.8+
dveditz: approval1.8.0.14+
roc: approval1.9+
Details | Diff | Splinter Review

Description Hwasung Kim 2007-07-27 00:02:13 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007072604 Minefield/3.0a7pre (Firefox 3)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7pre) Gecko/2007072604 Minefield/3.0a7pre (Firefox 3)

With a theme I metioned above, visiting a page contians a select element or opening the preferences window (which may contain dropdown menus) causes Firefox to crash or to be unresponsive.

I commented out two lines (GtkOptionMenu::indicator_size and GtkOptionMenu::indicator_spacing) in gtkrc after I found the problem occured in 'moz_gtk_option_menu_get_metrics' function (in /widget/src/gtk2/gtk2drawing.c). Then no more crash.

Reproducible: Always

Steps to Reproduce:
1. change the gtk+ theme to a theme such as Nova, GSM, etc.
2. launch Minefield
3. visit a page contains a select element or open the preferences window
Actual Results:  
crash or being unresponsive

Expected Results:  
display normally

Breakpad Crash Reports:
d8e41d78-3b89-11dc-a4d7-001a4bd43ed6
66f1830f-3b7d-11dc-a9e1-001a4bd43ed6
dcb123ab-3b8a-11dc-aac2-001a4bd43e5c
0d0df6cc-3b8e-11dc-8d0d-001a4bd46e84
be0473eb-3b93-11dc-bb3d-001a4bd43ed6
b9adca70-3b8f-11dc-85e4-001a4bd43ed6
6dde5bee-3b96-11dc-9143-001a4bd43ef6
de24b097-3b97-11dc-a30b-001a4bd46e84
Comment 1 timeless 2007-07-29 15:10:45 PDT
UUID	66f1830f-3b7d-11dc-a9e1-001a4bd43ed6
Time	2007-07-26 06:37:49.840000-07:00
Build ID	2007072604
OS	Linux
OS Version	0.0.0 Linux 2.6.22-8-generic #1 SMP Thu Jul 12 15:59:45 GMT 2007 i686 GNU/Linux
CPU	x86
CPU Info	GenuineIntel family 2 model 2 stepping 4
Crash Reason	SIGABRT
Crash Address	0xffffe410
Stack of Crashing Thread

frame	signature
0	@0xffffe410
1	libc-2.6.so@0x2c1f0
2	libc-2.6.so@0x61e2b
3	libc-2.6.so@0x6d8fa
4	libglib-2.0.so.0.1307.0@0x36960
5	moz_gtk_option_menu_get_metrics
6	moz_gtk_widget_paint
7	ThemeRenderer::NativeDraw(_XDisplay*, unsigned long, Visual*, short, short, XRectangle*, unsigned int)
8	NativeRendering(void*, _XDisplay*, unsigned long, Visual*, short, short, XRectangle*, unsigned int)
9	cairo_draw_with_xlib
Comment 2 Christian Persch (GNOME) (away; not receiving bug mail) 2007-07-29 15:41:22 PDT
565 GtkBorder *tmp_indicator_spacing;
566                 
567 gtk_widget_style_get(gOptionMenuWidget,
[...]
570                      "indicator_spacing", &tmp_indicator_spacing,
[...]
585 g_free(tmp_indicator_spacing);

You have to use gtk_border_free() to free a GtkBorder* obtained with gtk_widget_style_get, not g_free().
Comment 3 Christian Persch (GNOME) (away; not receiving bug mail) 2007-07-29 15:49:58 PDT
And similarly you need to use gtk_requisition_free to free the GtkRequisition* tmp_indicator_size.
Comment 4 timeless 2007-07-29 15:53:15 PDT
Created attachment 274406 [details] [diff] [review]
like this?
Comment 5 timeless 2007-07-29 15:54:51 PDT
Created attachment 274408 [details] [diff] [review]
more like this?
Comment 6 Christian Persch (GNOME) (away; not receiving bug mail) 2007-07-29 16:04:23 PDT
Comment on attachment 274408 [details] [diff] [review]
more like this?

You need to null-check, since neither they don't accept null, in contrast to g_free.
Comment 7 timeless 2007-07-29 16:08:37 PDT
Created attachment 274410 [details] [diff] [review]
details...
Comment 8 Christian Persch (GNOME) (away; not receiving bug mail) 2007-07-29 16:17:10 PDT
Comment on attachment 274410 [details] [diff] [review]
details...

You mixed them up, used gtk_border_free in the if() of the GtkRequisition, and likewise for the other one too ;)
Comment 9 timeless 2007-07-29 23:19:02 PDT
Created attachment 274430 [details] [diff] [review]
ok, patching whlie sleepy is bad?
Comment 10 timeless 2007-07-30 05:57:23 PDT
Comment on attachment 274430 [details] [diff] [review]
ok, patching whlie sleepy is bad?

roc: could you please approve this for 1.9?
Comment 11 Christian Persch (GNOME) (away; not receiving bug mail) 2007-09-04 13:31:21 PDT
*** Bug 394876 has been marked as a duplicate of this bug. ***
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-09-04 16:49:20 PDT
I'd like to but I technically I can't.
Comment 13 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-09-04 18:19:35 PDT
Comment on attachment 274430 [details] [diff] [review]
ok, patching whlie sleepy is bad?

okay, now I can.
Comment 14 Frederic Crozat 2007-09-05 00:07:25 PDT
attachment 274430 [details] [diff] [review] is buggy :
-there is a typo in gtk_requistion_free(tmp_indicator_size); and from checking gtk code, I'm not sure there is a need to use gtk_requisition_free, since it is just a  call to g_free. Anyway, it is not needed to check for null when calling gtk_requisition_free
-gtk_requisition_free on gtk+ 2.11.x is calling g_slide_free which does accept NULL as a parameter (it just doesn't do anything). 
Comment 15 Christian Persch (GNOME) (away; not receiving bug mail) 2007-09-05 11:12:16 PDT
You're right, I read the gslice macros wrongly and thought they didn't accept NULL on free.

gtk_requisition_free is the right function to call, NOT g_free.
Comment 16 Frederic Crozat 2007-09-06 00:44:14 PDT
Created attachment 279879 [details] [diff] [review]
better patch
Comment 17 Reed Loden [:reed] (use needinfo?) 2007-09-20 08:13:22 PDT
Comment on attachment 279879 [details] [diff] [review]
better patch

Requesting review on this patch... the patch says it's for gfx/src/gtk/gtk2drawing.c, but that file doesn't exist on trunk, as it seems it was moved to widget/src/gtk2/gtk2drawing.c.
Comment 18 Reed Loden [:reed] (use needinfo?) 2007-09-20 08:15:43 PDT
According to https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/129007, this is a top-crasher for Ubuntu.
Comment 19 Reed Loden [:reed] (use needinfo?) 2007-09-20 19:14:48 PDT
Checking in widget/src/gtk2/gtk2drawing.c;
/cvsroot/mozilla/widget/src/gtk2/gtk2drawing.c,v  <--  gtk2drawing.c
new revision: 1.32; previous revision: 1.31
done
Comment 20 Daniel Veditz [:dveditz] 2007-09-28 16:35:49 PDT
Comment on attachment 279879 [details] [diff] [review]
better patch

approved for 1.8.1.8 and 1.8.0.14, a=dveditz for release-drivers
Comment 21 Reed Loden [:reed] (use needinfo?) 2007-09-28 23:51:03 PDT
MOZILLA_1_8_BRANCH:

Checking in gfx/src/gtk/gtk2drawing.c;
/cvsroot/mozilla/gfx/src/gtk/Attic/gtk2drawing.c,v  <--  gtk2drawing.c
new revision: 1.15.8.3; previous revision: 1.15.8.2
done

MOZILLA_1_8_0_BRANCH:

Checking in gfx/src/gtk/gtk2drawing.c;
/cvsroot/mozilla/gfx/src/gtk/Attic/gtk2drawing.c,v  <--  gtk2drawing.c
new revision: 1.15.8.2.4.1; previous revision: 1.15.8.2
done

Note You need to log in before you can comment on or make changes to this bug.