Firefox crashes when gtk_key_theme (gconf) changes [@ nsLookAndFeel::InitColors]

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
Widget: Gtk
--
critical
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Matthias Clasen, Assigned: Adam Guthrie)

Tracking

({crash, fixed1.8.0.8, fixed1.8.1})

Trunk
mozilla1.9alpha1
x86
Linux
crash, fixed1.8.0.8, fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.6) Gecko/20060828 Fedora/1.5.0.6-8 Firefox/1.5.0.6 pango-text
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.6) Gecko/20060828 Fedora/1.5.0.6-8 Firefox/1.5.0.6 pango-text

Changing the value of /desktop/gnome/interface/gtk_key_theme (possible values
are Default and Emacs), firefox exits.

I haven't investigated why the crash occurs, but it seems dubious to connect
the same callback for key theme changes as for theme changes, since key theme
changes don't require any kind of screen refresh.

Just removing that callback for key theme changes should fix this.

Reproducible: Always

Steps to Reproduce:
1. start firefox
2. open gconf-editor and change /desktop/gnome/interface/gtk_key_theme


Actual Results:  
firefox crashes

Expected Results:  
firefox does not crash
(Assignee)

Comment 1

11 years ago
So, I'm wondering if this happens in a mozilla.org build. I don't see anywhere that we explicitly watch that key.

Also, can you get a stack for the crash? You'll need to get the firefox-debug package so you'll have symbols, then start with `firefox -g -d gdb'.
Keywords: crash
(Reporter)

Comment 2

11 years ago
Created attachment 239600 [details]
stacktrace
(Assignee)

Updated

11 years ago
Component: General → Widget: Gtk
Product: Firefox → Core
QA Contact: general → gtk
Summary: Firefox crashes when gtk_key_theme (gconf) changes → Firefox crashes when gtk_key_theme (gconf) changes [@ nsLookAndFeel::InitColors]
Version: unspecified → 1.8 Branch
(Assignee)

Comment 3

11 years ago
OK, so gtk_rc_get_style_by_paths() is probably returning NULL, although I don't know why since we're getting the style for a tooltip...

We should probably doing something more similar to this example: http://mail.gnome.org/archives/gtk-devel-list/2001-June/msg00449.html; e.g. actually creating the widget rather than explicitly passing hard-coded values to that function.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 4

11 years ago
I maintain that simply not doing anything like that for key theme changes is the easiest fix for the crash.
(Assignee)

Comment 5

11 years ago
That could be done fairly easily by commenting out this line: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/gtk2/nsWindow.cpp&rev=1.185&mark=2866-2868#2861

I'm not sure what the implications of doing this would be, though. dbaron?
Removing that is fine.  I don't think I knew what that was -- not sure where I got it from.

Still, it's not clear why it would crash.  Why don't we crash on theme changes?
(Assignee)

Comment 7

11 years ago
I really don't know. Maybe a bug in GTK? Matthias, can you get symbols for GTK, break in gtk_rc_get_style_by_paths and see what's going on?
(Reporter)

Comment 8

11 years ago
Thats really quite pointless. 
There is no reason to do this callback on a key theme change
Do you also crash on a theme change?
(Reporter)

Comment 10

11 years ago
No, on theme change, firefox goes into a looong (like >1min) near-death
experience, but recovers eventually...
(Assignee)

Comment 11

11 years ago
Created attachment 239612 [details] [diff] [review]
Remove callback for gtk-key-theme-change

OK. This removes the callback for gtk-key-theme-change(s). 

FWIW, Boris Zbarsky fixed the issue of the massive hanging when changing the GTK theme in bug 352096, but that patch is currently only on trunk.
Assignee: nobody → ispiked
Status: NEW → ASSIGNED
Attachment #239612 - Flags: superreview?(dbaron)
Attachment #239612 - Flags: review?(dbaron)
Comment on attachment 239612 [details] [diff] [review]
Remove callback for gtk-key-theme-change

r+sr=dbaron.  Thanks for fixing this.
Attachment #239612 - Flags: superreview?(dbaron)
Attachment #239612 - Flags: superreview+
Attachment #239612 - Flags: review?(dbaron)
Attachment #239612 - Flags: review+
(Assignee)

Updated

11 years ago
Whiteboard: [checkin needed]
(Assignee)

Comment 13

11 years ago
Checked in by timeless on 2006-09-22 00:24. ->FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Version: 1.8 Branch → Trunk
(Assignee)

Updated

11 years ago
Whiteboard: [checkin needed]

Comment 14

11 years ago
Comment on attachment 239612 [details] [diff] [review]
Remove callback for gtk-key-theme-change

mozilla/widget/src/gtk2/nsWindow.cpp 	1.186
Attachment #239612 - Attachment is obsolete: true
Comment on attachment 239612 [details] [diff] [review]
Remove callback for gtk-key-theme-change

Nominating. Either way, I really really want to add this crash fix to our RPMs if it doesn't make the branches.
Attachment #239612 - Flags: approval1.8.1?
Attachment #239612 - Flags: approval1.8.0.8?
This is safe.  I think we should take it for 1.8.1.
Comment on attachment 239612 [details] [diff] [review]
Remove callback for gtk-key-theme-change

a=mconnor on behalf of drivers for 1.8 branch checkin
Attachment #239612 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 239612 [details] [diff] [review]
Remove callback for gtk-key-theme-change

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #239612 - Flags: approval1.8.0.8? → approval1.8.0.8+
(Assignee)

Comment 19

11 years ago
(Eek, I guess it's my responsibility to get this checked in now.)
Whiteboard: [checkin needed (1.8 branch)][checkin needed (1.8.0 branch)]
(Assignee)

Comment 20

11 years ago
Comment on attachment 239612 [details] [diff] [review]
Remove callback for gtk-key-theme-change

This got approved on 2006-09-27, but didn't get checked in. dbaron recommended I re-request approval.

This is a simple GTK-only patch that Red Hat wants to get in for Firefox 2.
Attachment #239612 - Flags: approval1.8.1+ → approval1.8.1?
Checked in to MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.  (Forgot to change the name of the approver in the checkin comment for the latter; oops.)
Keywords: fixed1.8.0.8, fixed1.8.1
Whiteboard: [checkin needed (1.8 branch)][checkin needed (1.8.0 branch)]

Updated

11 years ago
Attachment #239612 - Flags: approval1.8.1? → approval1.8.1+
Crash Signature: [@ nsLookAndFeel::InitColors]
You need to log in before you can comment on or make changes to this bug.