Fix several issues with PidginAvatar in a conversation

Review Request #1689 — Created Aug. 30, 2022 and submitted

Information

pidgin/pidgin
default

Reviewers

  • The content-fit option is only in GTK 4.8+.
  • A static file can't be set as a GdkPixbuf, which causes warnings when
    hovering over the avatar.
  • The custom icon menu didn't work as it only checked for an attached buddy.
  • Changing the custom icon didn't cause a refresh in the buddy icon; this is
    fixed now, but in a slightly annoying way.
  • Changing the custom icon in the buddy list doesn't change the icon in the
    conversation window; this is not yet fixed due to missing signals.

The latter two can probably be fixed better when buddies/contacts have better
signals.

Hovered over avatar in a conversation, which didn't cause any warnings.
Picked all options in Custom Icon menu from a conversation, and they all worked as expected (except for a few buddy list / conversation interactions noted above.)

Also, only tested IM conversations; probably broken in chat conversations, since there are no checks for it?

Summary ID
Fix several issues with PidginAvatar in a conversation
* The `content-fit` option is only in GTK 4.8+. * A static file can't be set as a `GdkPixbuf`, which causes warnings when hovering over the avatar. * The custom icon menu didn't work as it only checked for an attached buddy. * Changing the custom icon didn't cause a refresh in the buddy icon; this is fixed now, but in a slightly annoying way. * Changing the custom icon in the buddy icon doesn't change the icon in the conversation window; this is not yet fixed due to missing signals. The latter two can probably be fixed better when buddies/contacts have better signals.
fc028f30e52a4e0bd0a8121b17f714f78e6ea761
Description From Last Updated

can't this just be replaced with get_effective_buddy ?

grimgrim

I'd rather just see this as an if since animation is initialized to NULL already.

grimgrim
grim
  1. 
      
  2. pidgin/pidginavatar.c (Diff revision 1)
     
     
    Show all issues

    can't this just be replaced with get_effective_buddy ?

    1. Different things are passed to the second argument of pidgin_avatar_find_buddy_icon, but that could maybe be done still.

  3. 
      
QuLogic
grim
  1. 
      
  2. pidgin/pidginavatar.c (Diff revisions 1 - 2)
     
     
    Show all issues

    I'd rather just see this as an if since animation is initialized to NULL already.

  3. 
      
QuLogic
grim
  1. Ship It!
  2. Awesome work, thank you so much!!

  3. 
      
grim
Review request changed
Status:
Completed