Ubuntu
virt-manager package

FFe: Support Application Indicators

Bug #525462 reported by Sense Egbert Hofstede
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
virt-manager
Won't Fix
Medium
virt-manager (Ubuntu)
Fix Released
Wishlist
Unassigned

Bug Description

Binary package hint: virt-manager

This application should be investigated to be ported to use Application
Indicators for Lucid - https://launchpad.net/indicator-application

Example code and the rationale behind Application Indicators can be found at <https://wiki.ubuntu.com/DesktopExperienceTeam/ApplicationIndicators>.

Sense Egbert Hofstede (sense)
tags: added: indicator-application
Changed in virt-manager (Ubuntu):
importance: Undecided → Wishlist
status: New → Triaged
Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Here is a first stab at application indicator support for virt-manager.

TODO:

- Revert to previous behaviour gracefully
- Find a way to load an icon from a path (may not be possible in python bindings)
- Figure out how to get menu to refresh without adding self.systray_icon.set_menu everywhere

Revision history for this message
Sense Egbert Hofstede (sense) wrote :

If I'm correct there is an appindicator.icon_path property, but that's the only place I could find anything related to icon paths.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Here is an updated patch.

TODO:
- Figure out how to get menu to refresh without adding self.systray_icon.set_menu everywhere

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

virt-manager package available for testing in my PPA:

https://launchpad.net/~mdeslaur/+archive/testing

Revision history for this message
In , Marc (marc-redhat-bugs-1) wrote :

Created attachment 395534
Initial version of application indicator support

Description of problem:

We would like to support application-indicators for virt-manager as
proposed at this page:

https://wiki.ubuntu.com/DesktopExperienceTeam/ApplicationIndicators

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

FFe Request:

Description of proposed changes:
- This debdiff adds Application Indicator support to virt-manager

Rationale:
- See https://wiki.ubuntu.com/DesktopExperienceTeam/ApplicationIndicators

Risks of regressions:
- Try icon is not enabled in default installation, it is a user preference.
- Code changes are minimal and can be easily fixed/reverted in case of regression.

summary: - Support Application Indicators
+ FFe: Support Application Indicators
Revision history for this message
Steve Langasek (vorlon) wrote :

FFe granted.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package virt-manager - 0.8.2-2ubuntu3

---------------
virt-manager (0.8.2-2ubuntu3) lucid; urgency=low

  * debian/patches/app_indicator_support.patch: Add Application Indicator
    support. (LP: #525462)
  * debian/virt-manager.links: link icon to where the Application Indicator
    can find it.
  * debian/control: add python-appindicator dependency
 -- Marc Deslauriers <email address hidden> Wed, 24 Feb 2010 17:33:10 -0500

Changed in virt-manager (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
In , Cole (cole-redhat-bugs) wrote :

Thanks for the patch, I've applied it upstream with one important change: lack
of the 'appindicator' module caused the app to fail to start. I would HIGHLY
recommend making use of the patch I applied upstream:

http://hg.fedorahosted.org/hg/virt-manager/rev/1cfeb4fd523d

Thanks for the contribution!

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

I need to revert this patch for now until bug #530138 is resolved as the workaround in the patch is causing massive amounts of ram to be used.

Changed in virt-manager (Ubuntu):
status: Fix Released → Confirmed
Sense Egbert Hofstede (sense)
Changed in virt-manager (Ubuntu):
status: Confirmed → Triaged
Revision history for this message
Loïc Minier (lool) wrote :

I don't think this patch was actually reverted, can we close this bug?

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Yes, the patch wasn't applied in the final version that shipped in Lucid.

Unfortunately, the patch made it upstream, so it will need to be disabled in the newer virt-manager that is now in maverick.

Revision history for this message
Sense Egbert Hofstede (sense) wrote :

Can't we just raise the priority of bug #530138 and make sure it is fixed in time for Maverick Alpha 2?

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package virt-manager - 0.8.4-3ubuntu4

---------------
virt-manager (0.8.4-3ubuntu4) maverick; urgency=low

  * Re-enable appindicator support. (LP: #525462)
    - debian/patches/disable-appindicator.patch: removed now that
      LP: #530138 has been resolved.
    - debian/patches/remove-appindicator-workarounds.patch: remove
      workarounds that impact performance as they are unnecessary now
      that LP: #530138 has been resolved.
 -- Marc Deslauriers <email address hidden> Fri, 09 Jul 2010 12:24:17 -0400

Changed in virt-manager (Ubuntu):
status: Triaged → Fix Released
Bug Watch Updater (bug-watch-updater)
Changed in virt-manager:
importance: Unknown → Medium
status: Unknown → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.