After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 739111 - GtkFontChooserDialog fixes
GtkFontChooserDialog fixes
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkFontChooser
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2014-10-24 08:57 UTC by Christophe Fergeau
Modified: 2014-10-27 01:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fontchooserwidget: Don't invalidate priv->font_iter in load_fonts (2.23 KB, patch)
2014-10-24 08:58 UTC, Christophe Fergeau
committed Details | Review
Return correct font from gtk_font_chooser_widget_find_font (2.38 KB, patch)
2014-10-24 08:58 UTC, Christophe Fergeau
committed Details | Review

Description Christophe Fergeau 2014-10-24 08:57:59 UTC
These patches fix 2 issues with GtkFontChooser.
The first one fixes selection of the current font when opening multiple times
a GtkFontChooserDialog from a GtkFontChooserButton (eg from gtk3-demo).

The second one fixes a recent regression in load_fonts.
Comment 1 Christophe Fergeau 2014-10-24 08:58:04 UTC
Created attachment 289259 [details] [review]
fontchooserwidget: Don't invalidate priv->font_iter in load_fonts

When using GtkFontChooserButton, the same GtkFontChooserWidget can be
hidden and shown multiple times. When doing that, the font that was
chosen the previous time should be the selected one in the
GtkFontChooserWidget, however this does not work as expected and a
somehow 'random' font gets selected (or none) instead.

Every time the font chooser widget is shown, its style will be updated,
causing gtk_font_chooser_widget_style_updated and then
gtk_font_chooser_widget_load_fonts to be called.

gtk_font_chooser_widget_load_fonts starts by clearing the GtkListStore
listing the available fonts, repopulates it, and then makes sure the
current font is selected.

However, this does not work as expected, as during the call to
gtk_list_store_clear, the cursor_changed_cb will be invoked multiple
times when the GtkTreeView cursor gets moved when the line where the
cursor currently is gets removed. This will cause the 'current font'
state (priv->font_desc) to be unexpectedly modified, and when
gtk_font_chooser_widget_load_fonts tries to reposition the cursor to the
'current font', we won't get the expect result.

This commit avoids that by making sure cursor_changed_cb does not get
called when we call gtk_list_store_clear in
gtk_font_chooser_widget_load_fonts.
Comment 2 Christophe Fergeau 2014-10-24 08:58:12 UTC
Created attachment 289260 [details] [review]
Return correct font from gtk_font_chooser_widget_find_font

Commit 30a1c4ab fixed several memleaks including one in
gtk_font_chooser_widget_find_font.

However, the fix causes one extra call to gtk_tree_model_iter_next()
after finding the font we look for (ie pango_font_description_equal
returns TRUE): the 'increment' part of the for loop
(gtk_tree_model_iter_next) is run before the 'exit condition' of the for
loop is evaluated.

This commit reverts this part of commit 30a1c4ab and adds an extra
call to pango_font_description_free in order to fix the leak.
Comment 3 Matthias Clasen 2014-10-25 17:21:56 UTC
Review of attachment 289259 [details] [review]:

ok
Comment 4 Matthias Clasen 2014-10-25 17:23:40 UTC
Review of attachment 289260 [details] [review]:

oops, thanks for catching this. I believe this needs to go on the gtk-3-14 branch too
Comment 5 Matthias Clasen 2014-10-27 01:56:15 UTC
Attachment 289259 [details] pushed as 77487fe - fontchooserwidget: Don't invalidate priv->font_iter in load_fonts
Attachment 289260 [details] pushed as 506d59f - Return correct font from gtk_font_chooser_widget_find_font