Merge lp:~mikemc/ubuntuone-control-panel/fix-native-dialog into lp:ubuntuone-control-panel

[Mike McCracken]

- Use applescript on OS X to show native file-chooser dialog. (LP: #1044201)

- Use applescript on OS X to show native file-chooser dialog. (LP: #1044201)

Adds a darwin-specific class to replace QFileDialog.getExistingDirectory which calls 'osascript' via subprocess, to pass an applescript on the command line. The script simply shows a file picker using the System Events pseudo-application, and prints the result (or returns an error code if the user cancels).

Tests pass on darwin and linux.

To test IRL, start control panel and press the "add a folder" button. It should be non-ugly.

[Michał Karnicki (karni)]

diff line 201 - intentional comment?
Looks good to me, can't test (no mac).

review: Approve

[lp:~mikemc/ubuntuone-control-panel/fix-native-dialog]

395. By Mike McCracken on 2012-12-21

re-add test cleanup that crashes test suite on darwin. Commenting it out is a hack that shouldn't end up in trunk, probably.

[dobey (dobey)]

259 + # Invalid name "getExistingDirectory"
260 + # pylint: disable=C0103

Can you drop the pylint comments you add here, as we're not using it any more?

review: Needs Fixing

[lp:~mikemc/ubuntuone-control-panel/fix-native-dialog]

396. By Mike McCracken on 2013-01-11

Strip pylint comments from moved FakedFileDialog

[dobey (dobey)]

Looks ok here.

review: Approve

=== modified file 'ubuntuone/controlpanel/gui/qt/addfolder.py'
--- ubuntuone/controlpanel/gui/qt/addfolder.py	2012-08-24 20:38:16 +0000
+++ ubuntuone/controlpanel/gui/qt/addfolder.py	2013-01-11 17:15:25 +0000
@@ -39,11 +39,18 @@
 # time only, pinky-swear, cross my heart and hope to die if
 # I lie.
 FILE_CHOOSER_OPTIONS = QtGui.QFileDialog.ShowDirsOnly
-if sys.platform in ('win32', 'darwin'):
+if sys.platform in ('win32'):
     FILE_CHOOSER_OPTIONS |= QtGui.QFileDialog.DontUseNativeDialog
 # Yes, that is true. If you do that again, you will be hunted
 # down and taught a lesson. You will be sorry.
 
+if sys.platform == 'darwin':
+    from ubuntuone.controlpanel.gui.qt.main.darwin import (
+        DarwinFileDialog)
+    FileDialog = DarwinFileDialog
+else:
+    FileDialog = QtGui.QFileDialog
+
 
 class AddFolderButton(cache.Cache, QtGui.QPushButton):
     """The AddFolderButton widget"""
@@ -68,9 +75,10 @@
         """The 'Sync another folder' button was clicked."""
         # The options argument is because of LP: #835013
         home_dir = yield self.backend.get_home_dir()
-        folder = QtGui.QFileDialog.getExistingDirectory(
-            parent=self, directory=home_dir,
-            options=FILE_CHOOSER_OPTIONS)
+        folder = FileDialog.getExistingDirectory(parent=self,
+                                                 directory=home_dir,
+                                                 options=FILE_CHOOSER_OPTIONS)
+        self.window().raise_()
         folder = unicode(QtCore.QDir.toNativeSeparators(folder))
         logger.info('on_add_folder_button_clicked: user requested folder '
                     'creation for path %r.', folder)

=== modified file 'ubuntuone/controlpanel/gui/qt/main/__init__.py'
--- ubuntuone/controlpanel/gui/qt/main/__init__.py	2012-12-08 00:13:34 +0000
+++ ubuntuone/controlpanel/gui/qt/main/__init__.py	2013-01-11 17:15:25 +0000
@@ -40,7 +40,8 @@
 
 if sys.platform == 'darwin':
     from ubuntuone.controlpanel.gui.qt.main.darwin import (
-        install_platform_event_handlers, MenubarIconLauncher)
+        install_platform_event_handlers,
+        MenubarIconLauncher)
     assert(install_platform_event_handlers)
     assert(MenubarIconLauncher)
 else:

=== modified file 'ubuntuone/controlpanel/gui/qt/main/darwin.py'
--- ubuntuone/controlpanel/gui/qt/main/darwin.py	2012-11-08 17:44:01 +0000
+++ ubuntuone/controlpanel/gui/qt/main/darwin.py	2013-01-11 17:15:25 +0000
@@ -35,6 +35,27 @@
 logger = setup_logging('qt.main.darwin')
 
 
+class DarwinFileDialog(object):
+    FOLDER_CHOOSER_SCRIPT = """
+    tell application "System Events"
+    activate
+    set home to POSIX file "%s" as alias
+    POSIX path of( choose folder default location home )
+    end tell
+    """
+
+    @classmethod
+    def getExistingDirectory(cls, directory=None, **kwargs):
+        """Use command-line applescript to avoid QTBUG-28146."""
+        try:
+            s = cls.FOLDER_CHOOSER_SCRIPT % directory
+            folder = subprocess.check_output(['osascript', '-e', s]).strip()
+        except subprocess.CalledProcessError as e:
+            logger.error("Could not call script to show open panel: %r" % e)
+            folder = ''
+        return folder
+
+
 @inlineCallbacks
 def handle_cmd_q(app, event, should_quit_sd=False):
     """Handle the quit event on darwin."""

=== modified file 'ubuntuone/controlpanel/gui/qt/main/tests/test_darwin.py'
--- ubuntuone/controlpanel/gui/qt/main/tests/test_darwin.py	2012-11-08 17:44:01 +0000
+++ ubuntuone/controlpanel/gui/qt/main/tests/test_darwin.py	2013-01-11 17:15:25 +0000
@@ -86,6 +86,46 @@
         self.handlers = []
 
 
+class ExternalGetDirectoryTestCase(TestCase):
+    """Test calling applescript to show file picker."""
+
+    @inlineCallbacks
+    def setUp(self):
+        """Simplify applescript for test."""
+        yield super(ExternalGetDirectoryTestCase, self).setUp()
+        self.patch(main.darwin.DarwinFileDialog,
+                   "FOLDER_CHOOSER_SCRIPT",
+                   "testscript: %s")
+        self.patch(main.darwin.subprocess, 'check_output',
+                   self._fake_check_output)
+
+        self.args = []
+        self.fake_raises = False
+
+    def _fake_check_output(self, args):
+        """mock for subprocess.check_output"""
+        self.args += args
+        if self.fake_raises:
+            raise subprocess.CalledProcessError(-1, 'fakecmd')
+        return "testpath"
+
+    def test_path_ok(self):
+        """Test returning the path."""
+
+        rv = main.darwin.DarwinFileDialog.getExistingDirectory(
+            directory="HOME")
+        self.assertEqual(rv, "testpath")
+        self.assertEqual(self.args, ["osascript", "-e", "testscript: HOME"])
+
+    def test_path_raises(self):
+        """Test returning the path."""
+        self.fake_raises = True
+        rv = main.darwin.DarwinFileDialog.getExistingDirectory(
+            directory="HOME")
+        self.assertEqual(rv, "")
+        self.assertEqual(self.args, ["osascript", "-e", "testscript: HOME"])
+
+
 class QuitTestCase(TestCase):
     """Test quit event handling."""
 

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/__init__.py'
--- ubuntuone/controlpanel/gui/qt/tests/__init__.py	2012-10-23 14:24:16 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/__init__.py	2013-01-11 17:15:25 +0000
@@ -17,7 +17,6 @@
 """The test suite for the Qt UI for the control panel for Ubuntu One."""
 
 import logging
-import os
 
 from PyQt4 import QtGui, QtCore
 from twisted.internet import defer
@@ -85,17 +84,6 @@
 NO_OP = lambda *args: None
 
 
-def set_path_on_file_dialog(folder_name=None, file_dialog=None):
-    """Set a valid path in the file dialog to create a folder."""
-    if folder_name is None:
-        folder_name = 'Documents'
-    folder = os.path.join(USER_HOME, folder_name)
-    if file_dialog is None:
-        file_dialog = FakedFileDialog
-    file_dialog.response = QtCore.QString(folder)
-    return folder
-
-
 class FakeUi(FakedObject):
     """A fake Ui object."""
 
@@ -264,30 +252,6 @@
         FakedDialog.properties['shown'] += 1
 
 
-class FakedFileDialog(object):
-    """Fake a file chooser dialog."""
-
-    response = args = kwargs = None
-    DontUseNativeDialog = object()
-
-    @classmethod
-    def reset(cls, *a, **kw):
-        """Clear all values."""
-        cls.response = cls.args = cls.kwargs = None
-
-    # Invalid name "getExistingDirectory"
-    # pylint: disable=C0103
-
-    @classmethod
-    def getExistingDirectory(cls, *a, **kw):
-        """Simulate a directory chooser."""
-        cls.args = a
-        cls.kwargs = kw
-        return cls.response
-
-    # pylint: enable=C0103
-
-
 class BaseTestCase(TestCase):
     """Base Test Case."""
 
@@ -326,10 +290,6 @@
             logger.addHandler(self.memento)
             self.addCleanup(logger.removeHandler, self.memento)
 
-        # default response if user does nothing
-        FakedFileDialog.reset()
-        self.patch(QtGui, 'QFileDialog', FakedFileDialog)
-
         FakedDialog.reset()
         self.patch(QtGui, 'QMessageBox', FakedDialog)
 

=== modified file 'ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py'
--- ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py	2012-03-19 20:55:52 +0000
+++ ubuntuone/controlpanel/gui/qt/tests/test_addfolder.py	2013-01-11 17:15:25 +0000
@@ -18,6 +18,10 @@
 
 """Tests for the AddFolderButton widget."""
 
+import os
+
+from PyQt4 import QtCore
+
 from twisted.internet import defer
 
 from ubuntuone.controlpanel.gui.tests import (
@@ -28,9 +32,7 @@
     BaseTestCase,
     CrashyBackend,
     CrashyBackendException,
-    FakedDialog,
-    FakedFileDialog,
-    set_path_on_file_dialog,
+    FakedDialog
 )
 
 # Access to a protected member
@@ -38,6 +40,36 @@
 # pylint: disable=W0212, E1103
 
 
+def set_path_on_file_dialog(folder_name=None, file_dialog=None):
+    """Set a valid path in the file dialog to create a folder."""
+    if folder_name is None:
+        folder_name = 'Documents'
+    folder = os.path.join(USER_HOME, folder_name)
+    if file_dialog is None:
+        file_dialog = FakedFileDialog
+    file_dialog.response = QtCore.QString(folder)
+    return folder
+
+
+class FakedFileDialog(object):
+    """Fake a file chooser dialog."""
+
+    response = args = kwargs = None
+    DontUseNativeDialog = object()
+
+    @classmethod
+    def reset(cls, *a, **kw):
+        """Clear all values."""
+        cls.response = cls.args = cls.kwargs = None
+
+    @classmethod
+    def getExistingDirectory(cls, *a, **kw):
+        """Simulate a directory chooser."""
+        cls.args = a
+        cls.kwargs = kw
+        return cls.response
+
+
 class AddFolderButtonTestCase(BaseTestCase):
     """The test suite for the AddFolderButton."""
 
@@ -47,6 +79,12 @@
     def setUp(self):
         yield super(AddFolderButtonTestCase, self).setUp()
         self.created_folders = []
+
+        # default response if user does nothing
+        FakedFileDialog.reset()
+
+        self.patch(gui, 'FileDialog', FakedFileDialog)
+
         self.patch(FakedFileDialog, 'response', gui.QtCore.QString(''))
         self.patch(self.ui.backend, 'validate_path_for_folder', lambda p: True)
         self.patch(self.ui, 'add_folder_func', self.add_folder)