Replace PidginAccountsDisabledMenu with PidginInactiveAccountsMenu which is a GMenuModel subclass.

Review Request #1366 — Created March 22, 2022 and submitted

Information

pidgin/pidgin
default

Reviewers

This gives us a bit more control as we are always in control of the attributes,
so we don't need to modify the menu in place. This also made it trivial to add
the protocol icons which are very helpful when you have the same user name on
multiple protocols.

Enabled and disabled two accounts a lot while under valgrind.

Summary ID
Reimplement the PidginAccountsDisabledMenu to be a GMenuModel subclass.
This gives us a bit more control as we are always in control of the attributes, so we don't need to modify the menu in place. This also made it trivial to add the protocol icons which are very helpful when you have the same user name on multiple protocols.
f54207e24fef2f6921106ca24ab72f419b02d28b
Description From Last Updated

Should it be disabled? Or maybe inactive?

QuLogicQuLogic

Not a Menu any more; probably easier to keep the type out of the docs here.

QuLogicQuLogic

This leaked the existing menu->accounts list.

QuLogicQuLogic

The key doesn't seem to need to be strdup'd, unless that's required of the API?

QuLogicQuLogic

Should use the attribute name constants.

QuLogicQuLogic

I'm no sure if this should be g_variant_ref_sink instead.

QuLogicQuLogic

Add space.

QuLogicQuLogic

Update type in comment.

QuLogicQuLogic
QuLogic
  1. 
      
  2. libpurple/purpleaccountmanager.h (Diff revision 1)
     
     
    Show all issues

    Should it be disabled? Or maybe inactive?

    1. Well we have "enabled" already, so I went with "disabled" but typoed. "inactive" I dunno?

  3. pidgin/pidginaccountsdisabledmenu.h (Diff revision 1)
     
     
    Show all issues

    Not a Menu any more; probably easier to keep the type out of the docs here.

  4. pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
     
     
    Show all issues

    This leaked the existing menu->accounts list.

  5. pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
     
     
    Show all issues

    The key doesn't seem to need to be strdup'd, unless that's required of the API?

  6. pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    Should use the attribute name constants.

  7. pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
     
     
    Show all issues

    I'm no sure if this should be g_variant_ref_sink instead.

  8. pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
     
     
    Show all issues

    Add space.

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

    Update type in comment.

  3. 
      
grim
QuLogic
  1. Ship It!
  2. 
      
grim
Review request changed
Status:
Completed