New location bar style does not update on GTK theme change

RESOLVED FIXED in mozilla1.9beta3

Status

()

Core
Widget: Gtk
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Ian Spence, Assigned: Teune van Steeg)

Tracking

Trunk
mozilla1.9beta3
x86
Linux
Points:
---
Bug Flags:
blocking1.9 ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

11 years ago
If you change the GTK theme, everything updates correctly except for the location bar and the new bookmark toolbar arrows.


I believe the culprit can be found in attachment 296147 [details] [diff] [review].

How code gets executed (when ensure_dropdown_entry_widget is called)
------------------
Call ensure_combo_box_entry_widget()
    Initialize gComboBoxEntry
    Set gComboBoxEntry as child of gProtoWindow
Initialize gDropdownEntryWidget
Set Set gComboBoxEntry as child of gDropdownEntryWidget
Realize gDropdownEntryWidget
-------------------

Notice that the final result of this is that gDropdownEntryWidget has no parent.  Thus it is never notified of a theme change, and never passes it on down to its child.

I think the fix is to replace:

gtk_widget_realize(gDropdownEntryWidget);

with:

setup_widget_prototype(gDropdownEntryWidget);
Blocks: 405210
(Assignee)

Comment 1

11 years ago
(In reply to comment #1)
You are right about where the culprit lies, but you are wrong at what the fix should be:

> Initialize gDropdownEntryWidget
> Set Set gComboBoxEntry as child of gDropdownEntryWidget

It's the other way around: gDropdownEntryWidget is set as child of gComboBoxEntry.
This is the same for the drop down button. So both widgets actually have parents.

The problem is that gtk_widget_set_parent does set up the correct child-parent relationship, but it doesn't work for theme changes apparently.

gComboBoxEntry must be the parent of both widgets to get them the right styling. Normally we would use gtk_container_add to make a widget a parent of another, but that doesn't work for the buttons of GtkComboBoxEntry widgets. It works for the GtkEntry widget, but then the styling for the entry of the location bar gets updated and not for the button when one changes themes, which looks very strange. 

To solve this, there must be a way to get the button (an internal child) from the GtkCombBoxEntry widget directly, for which I can't find a way to do that, or there has to be some kind of code run after gtk_widget_parent_set, which I haven't found either thus far. Also maybe this can be solved by setting the parent-child relation with another method.
Would roc know? or do we have some GTK person we can ask?
Flags: blocking1.9?
(Assignee)

Comment 3

11 years ago
Created attachment 298127 [details] [diff] [review]
use already created child widgets of GtkComboBoxEntry 

This patch fixes this bug: since AFAIK we can't add the button to GtkComboBoxEntry in a way so that it gets updated on theme changes, we must get the button which is already a part of GtkComboBoxEntry. Because it is an internal child, this is the only way I know of to get to it. 

Memory wise, I'm not sure if the second GList needs to be freed by g_list_free.
Assignee: nobody → twanno
Status: NEW → ASSIGNED
Attachment #298127 - Flags: superreview?(roc)
Attachment #298127 - Flags: review?(roc)
(Assignee)

Comment 4

11 years ago
Created attachment 298130 [details] [diff] [review]
use already created child widgets of GtkComboBoxEntry

Same fix; less code. This makes my doubts about correctly freeing the GList in my previous comment obsolete.
Attachment #298127 - Attachment is obsolete: true
Attachment #298130 - Flags: superreview?(roc)
Attachment #298130 - Flags: review?(roc)
Attachment #298127 - Flags: superreview?(roc)
Attachment #298127 - Flags: review?(roc)
(Assignee)

Comment 5

11 years ago
Created attachment 298131 [details] [diff] [review]
use already created child widgets of GtkComboBoxEntry

And now without the code that wasn't supposed to be in the patch
Sorry about that :(
Attachment #298130 - Attachment is obsolete: true
Attachment #298131 - Flags: superreview?(roc)
Attachment #298131 - Flags: review?(roc)
Attachment #298130 - Flags: superreview?(roc)
Attachment #298130 - Flags: review?(roc)
Attachment #298131 - Flags: superreview?(roc)
Attachment #298131 - Flags: superreview+
Attachment #298131 - Flags: review?(roc)
Attachment #298131 - Flags: review+
Comment on attachment 298131 [details] [diff] [review]
use already created child widgets of GtkComboBoxEntry

Fix regression with changing themes on Linux.
Attachment #298131 - Flags: approval1.9?

Updated

11 years ago
Attachment #298131 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in widget/src/gtk2/gtk2drawing.c;
/cvsroot/mozilla/widget/src/gtk2/gtk2drawing.c,v  <--  gtk2drawing.c
new revision: 1.70; previous revision: 1.69
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
You need to log in before you can comment on or make changes to this bug.