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 452861 - gtk_label_set_pattern() is not working anymore
gtk_label_set_pattern() is not working anymore
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.11.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 443776
 
 
Reported: 2007-07-01 14:49 UTC by Vincent Untz
Modified: 2007-07-09 20:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proof-of-concept patch (1.22 KB, patch)
2007-07-01 14:58 UTC, Vincent Untz
needs-work Details | Review
Update for comments from Matthias (2.15 KB, patch)
2007-07-04 15:59 UTC, Vincent Untz
committed Details | Review

Description Vincent Untz 2007-07-01 14:49:58 UTC
I believe this is the cause of bug 443776 (panel has needless mnemonic on the first char of menu).

With the addition of gtk-enable-mnemonics, a call to gtk_label_set_pattern() is overridden by the signal handler of "screen-changed". We should probably remember that gtk_label_set_pattern() was called and that the pattern shouldn't be overridden.

Here's a stack trace showing why gtk_label_set_pattern_internal() is called:

  • #0 gtk_label_set_pattern_internal
    at gtklabel.c line 1538
  • #1 gtk_label_set_uline_text_internal
    at gtklabel.c line 2651
  • #2 gtk_label_recalculate
    at gtklabel.c line 1220
  • #3 label_shortcut_setting_apply
    at gtklabel.c line 976
  • #4 gtk_label_screen_changed
    at gtklabel.c line 1040
  • #5 IA__g_cclosure_marshal_VOID__OBJECT
    at gmarshal.c line 636
  • #6 g_type_class_meta_marshal
    at gclosure.c line 567
  • #7 IA__g_closure_invoke
    at gclosure.c line 490
  • #8 signal_emit_unlocked_R
    at gsignal.c line 2478
  • #9 IA__g_signal_emit_valist
    at gsignal.c line 2199
  • #10 IA__g_signal_emit
    at gsignal.c line 2243
  • #11 do_screen_change
    at gtkwidget.c line 5656
  • #12 gtk_widget_propagate_hierarchy_changed_recurse
    at gtkwidget.c line 5678
  • #13 gtk_menu_item_forall
    at gtkmenuitem.c line 1640
  • #14 gtk_image_menu_item_forall
    at gtkimagemenuitem.c line 350
  • #15 IA__gtk_container_forall
    at gtkcontainer.c line 1447
  • #16 gtk_widget_propagate_hierarchy_changed_recurse
    at gtkwidget.c line 5681
  • #17 gtk_menu_shell_forall
    at gtkmenushell.c line 973
  • #18 IA__gtk_container_forall
  • #19 gtk_widget_propagate_hierarchy_changed_recurse
    at gtkwidget.c line 5681
  • #20 _gtk_widget_propagate_hierarchy_changed
    at gtkwidget.c line 5718
  • #21 IA__gtk_widget_set_parent
    at gtkwidget.c line 5111
  • #22 IA__gtk_fixed_put
    at gtkfixed.c line 165
  • #23 panel_widget_add
    at panel-widget.c line 2522

Comment 1 Vincent Untz 2007-07-01 14:58:52 UTC
Created attachment 90971 [details] [review]
Proof-of-concept patch

Wit this patch, everything works fine.

I'm not sure it's 100% right, though. Maybe gtk_label_set_uline_text_internal() should be changed to look at pattern_set.
Comment 2 Matthias Clasen 2007-07-02 17:20:06 UTC
That patch looks wrong to me. Shouldn't set_pattern() rather unset use_underline ?
Comment 3 Vincent Untz 2007-07-02 19:11:04 UTC
Putting "gtk_label_set_use_underline_internal (label, FALSE);" in set_pattern() results in this: "_Places" (with the underscore being a separated character).

Hrm. I'm not familiar at all with the code there, but something better than my first patch might be to not change effective_attrs when the pattern has been set.

It's also not clear to me what has the higher priority between the pattern property and the use-underline/use-markup properties. I'm assuming it's the pattern one, but I don't know if it's right.
Comment 4 Matthias Clasen 2007-07-02 19:26:36 UTC
Actually, looking at this mess and what the panel does some more,
the best way to support this may be to just make mnemonic-keyval
a writable property - we still need to do the extra work to ensure
that recalculating the label contents will not overwrite the custom
mnemonic-keyval, but that should be doable.
Comment 5 Matthias Clasen 2007-07-03 18:08:01 UTC
But maybe the approach in Vincents patch is good enough. Two things:

1) I believe the gtk_label_set_pattern (label, NULL) should reset
pattern_set to FALSE

2) GtkLabel is the most common widget, therefore I'd hate to grow it.
  Can we use one of the few remaining bits in the public struct instead ?
Comment 6 Vincent Untz 2007-07-04 15:59:32 UTC
Created attachment 91188 [details] [review]
Update for comments from Matthias

It probably makes sense to call gtk_label_recalculate() when unsetting the pattern with gtk_label_set_pattern (label, NULL), so I added this too.
Comment 7 Matthias Clasen 2007-07-09 20:54:42 UTC
2007-07-09  Matthias Clasen  <mclasen@redhat.com>

        * gtklabel.[ch]: Use a bit of the GtkLabel structure to
        remember that a pattern has been set.
        (gtk_label_set_pattern_internal): Don't do anything if
        a specific pattern has been set.
        (gtk_label_set_pattern): set the new bit to TRUE when
        setting a pattern, and recalculate everything if the
        pattern is unset. Fix gtk_label_set_pattern() not working
        anymore.  (#452861, Vincent Untz)