Support application indicators

Patch was made against upstream git tag BRASERO_2_29_6, matching the version in Lucid, and I have verified patch both cleanly applies against the original tarball acquired via apt-get source -d brasero (using patch -p0) and against git HEAD (via git rebase).

Feature complete with the following caveats. particularly to make it upstream acceptable:

- Probably need to add a flag to configure like
  --enable/disable-app-indicators. My autoconf foo is weak, so I left
  that undone.

- When compiling with app-indicator support,
  libbrasero-burn/brasero-tray is still compiled and linked, which is
  unnecessary. I tried added #ifdefs in Makefile.am around the lines
  which included brasero-tray.c and brasero-tray.h, but that didn't
  work.

- I have code duplication between the existing
  libbrasero-burn/brasero-tray.c and the new
  libbrasero-burn/brasero-app-indicator.c, due to a cut and paste
  job to get started. The areas I'm concerned about:

  - in libbrasero-burn/brasero-app-indicator.c, the function
    brasero_app_indicator_set_progress_menu_text (starting line 227)
    is largely a copy of libbrasero-burn/brasero-tray.c, the function
    brasero_tray_icon_set_tooltip (starting line 244), except for the
    portion where the menu item label is set vs. the original tooltip
    being set.
  - similarly, libbrasero-burn/brasero-app-indicator.c, function
    brasero_app_indicator_set_progress (starting line 281) is largely
    a copy of libbrasero-burn/brasero-tray.c, function
    brasero_tray_icon_set_progress (starting line 293), except for the
    calls to set the tooltip and set the icon.

  I wasn't sure how to address these the best. It didn't make sense
  to inherit BraseroAppIndicator from BraseroTrayIcon because
  BraseroTrayIcon itself is a sub-class of GtkStatusIcon. Should I
  just move the common calculation / string creation methods out to a
  separate .h/.c pair and call those from each? That leaves the
  question of where to free the string.

Any other suggestions would be great so I can improve my patches in
the future.

Will attach patch to upstream bug after Ken has had a chance to review it.

I made some changes to what you had in configure.in
 * Follow naming conventions already used by upstream
   - APPINDICATOR_REQUIRED instead of APPINDICATOR_REQS
   - enable_appindicator instead of have_app_indicator
 * Make it an enable arg like you suggested, default to yes

Not sure how to best remove the code duplication, so pending any other suggestions from Ken, likely ready for upstream review.

Change to proper state, per seb128.

This patch fixes a bug found by Ken by changing the "cancel" menu item to "close" to emulate the behavior of the old GtkStatusIcon. It also removes the icons from the menu items to make the menu formatting look correct.

Ready for re-review. Note that this change is as invasive as the change to vino, bug #497883, and perhaps should first be vetted upstream as Martin suggested for vino.

I reviewed the translated strings in the patch. Many are already present, but "Close" is new. Is it possible to use e. g. a stock button for that, or drop it entirely? The help string is also just "Close", it's not really obvious what it does -- close the menu, close the application, cancel the burn?

Please forward this patch to the upstream bug as well.

Also, I don't even see the "Close" in the menu. Instead, I only see an untranslated "Cancel" (but that string is upstream as well, German translation is just missing).

Otherwise this is working fine. I'll wait with the upload until the string question is settled and the patch gets forwarded upstream.

Thanks!

To address Martin's concerns, I discussed with mpt what the
appropriate user interaction here should be. I did the following:

- I reverted the cancel menu item back to using GTK_STOCK_CANCEL so it
  will be translated and then did some additional work to remove the
  icon from being displayed.

- Since the application indicator does not give any more information
  than the dialog does, I removed the change of the menu item from
  cancel -> close. Instead, when the burn is finished, the
  application indicator is hidden and the dialog is shown if hidden.
  When the dialog is destroyed, the application indicator is also
  destroyed.

Hopefully this has both made the interaction simpler and also removed
the string translation issue.

Thanks for the feedback Martin.

Thanks Travis! Looks great now, I'll upload this now. Please still forward it upstream.

This bug was fixed in the package brasero - 2.29.91-0ubuntu2

---------------
brasero (2.29.91-0ubuntu2) lucid; urgency=low

  * Add 012_appindicator.patch: Support application indicator, patch by Travis
    B. Hartwell. (LP: #497853)
  * debian/control.in: Add libappindicator build dependency.
  * Regenerate 99_autoconf.patch to pick up above patch.
 -- Martin Pitt <email address hidden> Thu, 04 Mar 2010 11:30:57 +0100