Ubuntu One Client

Merge lp://staging/~3v1n0/ubuntuone-client/sync-menu-use-gappinfo-launch into lp://staging/ubuntuone-client

sync-menu-use-gappinfo-launch
Merge into trunk

Proposed by Marco Trevisan (Treviño) on 2013-04-11

Status:

Merged

Approved by:

dobey on 2013-04-16

Approved revision:

1399

Merged at revision:

1393

Proposed branch:

lp://staging/~3v1n0/ubuntuone-client/sync-menu-use-gappinfo-launch

Merge into:

lp://staging/ubuntuone-client

Diff against target:

352 lines (+205/-48)

2 files modified

tests/platform/sync_menu/test_linux.py (+139/-20)
ubuntuone/platform/sync_menu/linux.py (+66/-28)

To merge this branch:

bzr merge lp://staging/~3v1n0/ubuntuone-client/sync-menu-use-gappinfo-launch

Related bugs:

Bug #627195: Window management - Apps raised from indicators sometimes dont have the focus	Undecided	Fix Committed
Bug #1168175: Opening the "Ubuntu One folder" from indicator-sync doesn't bring nautilus to focus	Medium	Fix Released

Link a bug report

Reviewer	Date Requested	Status
dobey (community)		Approve on 2013-04-16
Diego Sarmentero (community)		Approve on 2013-04-16
Roberto Alsina (community)	2013-04-11	Approve on 2013-04-12
Review via email: mp+158503@code.staging.launchpad.net

Commit message

UbuntuOneSyncMenuLinux: use GAppInfo with proper context to launch URIs

This allows to correctly open the application with proper startup notify.

Description of the change

The linux sync-menu should use GAppInfo with LaunchContext to launch applications and URIs so that the window manager and the applications will be properly focused and launched with startup notification.

Revision history for this message

Roberto Alsina (ralsina) on 2013-04-12:

review: Approve

lp://staging/~3v1n0/ubuntuone-client/sync-menu-use-gappinfo-launch updated on 2013-04-12

1393. By Marco Trevisan (Treviño) on 2013-04-12: UbuntuOneSyncMenuLinux: protect the commandline-app-creation from error too

Revision history for this message

Diego Sarmentero (diegosarmentero) wrote on 2013-04-12:

Tests are failing, and there isn't new tests for the new code.

review: Needs Fixing

Revision history for this message

dobey (dobey) wrote on 2013-04-12:

+CLIENT_DESKTOP_ID = 'ubuntuone-control-panel-qt-gnome.desktop'
+INSTALLER_DESKTOP_ID = 'ubuntuone-installer.desktop'

Why are both of these being used? There is no more "installer" exactly, but the filename is unchanged, to avoid breaking the launcher. Also, the "-qt-gnome.desktop" file is only for GNOME, so I think we want to use the "ubuntuone-installer.desktop" everywhere. But I wouldn't call it "INSTALLER_DESKTOP_ID" as the variable, as it's not an installer. Maybe CONTROL_PANEL_DESKTOP_ID or CLIENT_DESKTOP_ID, and use that constant for all the instances.

Revision history for this message

dobey (dobey) on 2013-04-12:

review: Needs Fixing

lp://staging/~3v1n0/ubuntuone-client/sync-menu-use-gappinfo-launch updated on 2013-04-12

1394. By Marco Trevisan (Treviño) on 2013-04-12: UbuntuOneSyncMenuLinux: don't crash if the display is not set, don't use the Context
1395. By Marco Trevisan (Treviño) on 2013-04-12: SyncMenuTestCase: add new tests to cover changes and update the old ones
1396. By Marco Trevisan (Treviño) on 2013-04-12: SyncMenuTestCase: use random values for timestasmp
1397. By Marco Trevisan (Treviño) on 2013-04-12: UbuntuOneSyncMenuLinux: always use CLIENT_DESKTOP_ID as client

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2013-04-12:

> +CLIENT_DESKTOP_ID = 'ubuntuone-control-panel-qt-gnome.desktop'
> +INSTALLER_DESKTOP_ID = 'ubuntuone-installer.desktop'
>
> Why are both of these being used? There is no more "installer" exactly, but
> the filename is unchanged, to avoid breaking the launcher. Also, the "-qt-
> gnome.desktop" file is only for GNOME, so I think we want to use the
> "ubuntuone-installer.desktop" everywhere. But I wouldn't call it
> "INSTALLER_DESKTOP_ID" as the variable, as it's not an installer. Maybe
> CONTROL_PANEL_DESKTOP_ID or CLIENT_DESKTOP_ID, and use that constant for all
> the instances.

Fine, I've moved everything to CLIENT_DESKTOP_ID set to "ubuntuone-installer.desktop" now.

Revision history for this message

dobey (dobey) wrote on 2013-04-15:

There are some lint issues in the test file.

== Python Lint Notices ==

./tests/platform/sync_menu/test_linux.py:
    276: redefinition of unused 'time' from line 32
    286: redefinition of unused 'time' from line 32
    297: redefinition of unused 'time' from line 32
    306: redefinition of unused 'time' from line 32
    317: redefinition of unused 'time' from line 32
    326: redefinition of unused 'time' from line 32
    335: redefinition of unused 'time' from line 32
    344: redefinition of unused 'time' from line 32
    353: redefinition of unused 'time' from line 32

make: *** [lint] Error 1

Revision history for this message

dobey (dobey) wrote on 2013-04-15:

Also, is there any reason to use use randint() here instead of time.time()?

Revision history for this message

Diego Sarmentero (diegosarmentero) wrote on 2013-04-15:

This branch contains some lint issues:

== Python Lint Notices ==

review: Needs Fixing

lp://staging/~3v1n0/ubuntuone-client/sync-menu-use-gappinfo-launch updated on 2013-04-15

1398. By Marco Trevisan (Treviño) on 2013-04-15: SyncMenuTestCase: use time.time() instead of random.randint().

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2013-04-15:

> This branch contains some lint issues:

They should be fixed now, thanks for pointing out.

Revision history for this message

Diego Sarmentero (diegosarmentero) wrote on 2013-04-16:

review: Approve

Revision history for this message

dobey (dobey) wrote on 2013-04-16:

8 +

E303 too many blank lines (3)

+ def __init__(self, command_line = "", name = "", flags = 0):
+ def __init__(self, desktop_id = ""):
+ def _open_uri(self, uri, timestamp = 0):
+ def _open_control_panel_by_command_line(self, timestamp, args = ''):
+ def open_control_panel(self, menuitem = None, timestamp = 0):
+ def open_ubuntu_one_folder(self, menuitem = None, timestamp = 0):
+ def open_share_file_tab(self, menuitem = None, timestamp = 0):
+ def open_go_to_web(self, menuitem = None, timestamp = 0):
+ def open_web_help(self, menuitem = None, timestamp = 0):
+ def open_get_more_storage(self, menuitem = None, timestamp = 0):