Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2900 closed Bug (fixed)

Indicator Application can only work with icon names, not rendered icons

Reported by: qense Owned by: charles
Priority: Normal Milestone: 1.90
Component: GTK+ Client Version: 1.83
Severity: Normal Keywords:
Cc:

Description (last modified by charles)

In ticket #2873 I submitted a patch that added Indicator Application support to Transmission. What I didn't realise when attaching the patch was that libappindicator cannot cope with generated images as tray icons, which are the icons used as quenfall-back mechanism by Transmission in case TRAY_ICON doesn't exist -- which is the case most of the times as it is now.

Attached is a patch that fixes the fall-back mechanism for the tray icon; in tr-icon.c the application first checks whether TRAY_ICON is available in the GTK+ icon theme -- the same check as used in actions.c for the default fall-back mechanism. If it doesn't exist the icon_name is set to "transmission", which does exist on all systems with Transmission installed.

(A bit unrelated: maybe it would be useful if a notice about Indicator Application support would be added to <http://trac.transmissionbt.com/wiki/Building> in the Ubuntu section. I've set the minimum libappindicator version to 0.0.11 in configure.ac.)

Attachments (3)

icon_fallback.patch (1.6 KB) - added by qense 12 years ago.
icon_fallback_maybe.patch (1.9 KB) - added by charles 12 years ago.
suggested revision of previous patch
icon_fallback_maybe_without_typo.patch (1.9 KB) - added by qense 12 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 12 years ago by charles

  • Component changed from Transmission to GTK+ Client
  • Owner set to charles

comment:2 Changed 12 years ago by charles

  • Keywords needinfo added
if( !gtk_icon_theme_has_icon( theme, TRAY_ICON ) ) { 
   icon_name = TRAY_ICON; 
} else { 
    icon_name = "transmission"; 
}

maybe I'm confused, but isn't that logic backwards? Wouldn't you want to use TRAY_ICON if gtk_icon_theme_has_icon( TRAY_ICON ) returns true?

comment:3 Changed 12 years ago by qense

I'm confused. With the admittedly wrong logic in the patch the icon is shown. However, when fixing the logic, or when assigning TRAY_ICON directly to 'icon_name' the icon doesn't work anymore. I'm investigating this further. Very strange...

Changed 12 years ago by qense

comment:4 Changed 12 years ago by qense

Updated the patch, this time it does work correctly.

The problem was that Transmission adds an icon to the icon theme when TRAY_ICON cannot be found. This made my code think that it existed, whereas Indicator Application couldn't find it.

I've solved this by letting the code checks if the icon has got a file patch associated with itself.

Changed 12 years ago by charles

suggested revision of previous patch

comment:5 Changed 12 years ago by charles

  • Description modified (diff)

qense: does that revised version work as well? if so I'll commit it for next week's release.

comment:6 Changed 12 years ago by qense

The patch works fine, thank you for improving it! However, there is a bug in it: you've forgot to change the variable name 'indicator_menu' to 'w' everywhere it is used, app_indicator_set_menu() was still using the old variable name. I'm attaching a fixed patch.

Changed 12 years ago by qense

comment:7 Changed 12 years ago by charles

Ha! I really should do this stuff when I'm -not- tired. :)

Thanks for the fix.

comment:8 Changed 12 years ago by charles

  • Status changed from new to assigned

Patch added to trunk for 1.90 by r10200

comment:9 Changed 12 years ago by charles

  • Keywords needinfo removed
  • Resolution set to fixed
  • Status changed from assigned to closed

comment:10 Changed 12 years ago by charles

  • Milestone changed from None Set to 1.90
Note: See TracTickets for help on using tickets.