Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13164 closed patch (fixed)

Editable comboboxes are implemented incorrectly

Reported by: 10110111 Owned by:
Milestone: 2.7.10 Component: pidgin (gtk)
Version: 2.7.1 Keywords:
Cc:

Description

Here's how it's implemented now:

ret = GTK_COMBO_BOX(gtk_combo_box_new_text()); the_entry = gtk_entry_new(); gtk_container_add(GTK_CONTAINER(ret), the_entry);

This doesn't look correctly with Oxygen-GTK style. See: Buddies -> Add Buddy -> "Add buddy to group" combobox.

Solution would be to use GtkComboboxEntry? instead.

Attachments (3)

shot-01.png (25.4 KB) - added by 10110111 8 years ago.
Screenshot
gtkutils.patch (666 bytes) - added by hpereiradacosta 8 years ago.
simple patch that fixes the issue
pidgin.png (41.6 KB) - added by hpereiradacosta 8 years ago.
'fixed' pidgin with oxygen-gtk

Download all attachments as: .zip

Change History (20)

Changed 8 years ago by 10110111

Screenshot

comment:1 Changed 8 years ago by rekkanoryo

  • Milestone set to Patches welcome

This is legacy from the days when we maintained compatibility with GTK+ 2.0.0. Patches to fix stuff like this are definitely welcome.

Changed 8 years ago by hpereiradacosta

simple patch that fixes the issue

comment:2 Changed 8 years ago by hpereiradacosta

patch attached. Is very simple and perfectly legit, from gtkcomboboxentry documentation. was tested against qtcurve (which had the same rendering issue) and oxygen-gtk.

also, could not find any 'feature' regression with respect to old implementation.

comment:3 Changed 8 years ago by hpereiradacosta

ps: patch was writtent for the 2.7.9 version of pidgin.

comment:4 Changed 8 years ago by rekkanoryo

  • Milestone changed from Patches welcome to 2.7.10

This needs a review before we release 2.7.10. I'm hoping someone beats me to it, but I'll do it if necessary.

comment:5 Changed 8 years ago by deryni

Can we get a screenshot of the dialog with the patch attached to compare to?

According to the docs on library.gnome.org both gtk_combo_box_new_text and gtk_combo_box_entry_new_text were added in GTK+ 2.4.

Do we know if other themes draw this correctly? Could this be a theme drawing bug, misfeature, or just plain ugly case?

Changed 8 years ago by hpereiradacosta

'fixed' pidgin with oxygen-gtk

comment:6 Changed 8 years ago by hpereiradacosta

See screenshot above for the fixed version. This is with oxygen-gtk. Latest QtCurve? shows the same issue, and gets fixed just the same by the patch. Though they represent the comboboxentries in similar ways, the code base is between both styles is _very_ different (if only because oxygen-gtk is in c++). So this is not a style bug.

The 'fundamental' issue, is that certain styles can decide do draw a GtkComboBoxEntry? in a different way than a (non editable) ComboBox? (especially for the button part).

Therefore: a GtkComboBox? (with a GtkEntry? on top of it), would not be drawn as a GtkComboBoxEntry? (as is intended).

This is best illustrated by comparing the first widget in the screenshot (the 'account'), to the last one (the 'group').

I hope this makes sense.

Finally, styles for which the combobox arrow-button is rendered the same for GtkComboBox? and GtkComboBox? entry 1/ were not affected by the bug (due to the above) 2/ are not affected by the patch, either.

Just tested with some other simple gtk styles (and can send screenshots too, if requested). Unfortunately, I don't have more modern styles/engines like e.g. clearlooks, or even murine, installed here, to test further.

comment:7 Changed 8 years ago by 10110111

Really nice patch, Hugo. I didn't even think it would be *that* easy :)

comment:8 Changed 8 years ago by deryni

The patch is certainly simple enough. It looks like it'd probably be fine to me. Those functions are used in very few places in pidgin directly (gtkblist.c and the crazychat and gevolution plugins). gtkblist.c and the gevolution plugin look like they will be fine with this change I'm less sure (from a quick read) that the crazy chat plugin will be (I don't know if its assumptions will still hold). That's the only concern I'd have about this, usages like the crazychat plugin breaking.

comment:9 Changed 8 years ago by rekkanoryo

Do we actually care about the crazychat plugin breaking? It's been completely unmaintained since the SoC student who wrote it disappeared.

comment:10 Changed 8 years ago by rekkanoryo

  • Type changed from defect to patch

This ticket should actually be a patch, since there's a patch attached. Makes my life easier :)

comment:11 Changed 8 years ago by deryni

I certainly don't, just figured I'd mention it.

I actually wonder if we need our combo box abstraction at all at this point (especially given the small number of places that use it).

comment:12 Changed 8 years ago by hpereiradacosta

"I actually wonder if we need our combo box abstraction at all at this point (...)" The only real difference I've seen with respect to GtkComboBoxEntry? is the behavior against pressing arrow_up or arrow_down. With pidgin's implementation, it opens the menu (rather: the list), whereas with the native implementation, it moves one item down/up in the (unopened) list, changing the text entry accordingly.

comment:13 Changed 8 years ago by deryni

Good to note. One of the things that made me wonder whether we wanted ours was whether we were duplicating functionality that now exists natively but whether we do things slightly differently is also important to think about.

comment:14 Changed 8 years ago by rekkanoryo

hpereiradacosta, how should you be credited for this patch? (At least name, but preferably e-mail address too.)

comment:15 Changed 8 years ago by hpereiradacosta

rekkanory:

Hugo Pereira Da Costa (yeah thats a long name) hugo@…

Thanks !

comment:16 Changed 8 years ago by hugo@…

  • Resolution set to fixed
  • Status changed from new to closed

(In 22adc92b81eaedd487463cb60a7651034680376d):
Modify the editable comboboxes to be more friendly to GTK+ themes that don't theme our previous method correctly. Fixes #13164.

comment:17 Changed 8 years ago by rekkanoryo@…

(In e8c1e8a79ad64bd73663a15cc0e802b06a8bc733):
Credit where due. Refs #13164.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!