Change how we define which menu items are included in the main menu on Mac OS

Change variable type to conform to expected type
Make vlcplayer stop() call asynchronous to avoid a deadlock with the UI thread
Add test for the main window function set_service_modified

bzr-revno: 2382

openlp/core/ui/mainwindow.py

@@ -320,12 +320,12 @@ class Ui_MainWindow(object):
         # i18n add Language Actions
         add_actions(self.settings_language_menu, (self.auto_language_item, None))
         add_actions(self.settings_language_menu, self.language_group.actions())
-        # Order things differently in OS X so that Preferences menu item in the
-        # app menu is correct (this gets picked up automatically by Qt).
+        # Qt on OS X looks for keywords in the menu items title to determine which menu items get added to the main
+        # menu. If we are running on Mac OS X the menu items whose title contains those keywords but don't belong in the
+        # main menu need to be marked as such with QAction.NoRole.
         if sys.platform == 'darwin':
-            add_actions(self.settings_menu, (self.settings_plugin_list_item, self.settings_language_menu.menuAction(),
-                        None, self.settings_configure_item, self.settings_shortcuts_item, self.formatting_tag_item))
-        else:
+            self.settings_shortcuts_item.setMenuRole(QtGui.QAction.NoRole)
+            self.formatting_tag_item.setMenuRole(QtGui.QAction.NoRole)
         add_actions(self.settings_menu, (self.settings_plugin_list_item, self.settings_language_menu.menuAction(),
                     None, self.formatting_tag_item, self.settings_shortcuts_item, self.settings_configure_item))
         add_actions(self.tools_menu, (self.tools_add_tool_item, None))

openlp/core/ui/media/vlcplayer.py

@@ -34,6 +34,7 @@ from distutils.version import LooseVersion
 import logging
 import os
 import sys
+import threading
 
 from PyQt4 import QtGui
 
@@ -207,7 +208,7 @@ class VlcPlayer(MediaPlayer):
         start_time = 0
         if self.state != MediaState.Paused and controller.media_info.start_time > 0:
             start_time = controller.media_info.start_time
-        display.vlc_media_player.play()
+        threading.Thread(target=display.vlc_media_player.play).start()
         if not self.media_state_wait(display, vlc.State.Playing):
             return False
         self.volume(display, controller.media_info.volume)
@@ -233,7 +234,7 @@ class VlcPlayer(MediaPlayer):
         """
         Stop the current item
         """
-        display.vlc_media_player.stop()
+        threading.Thread(target=display.vlc_media_player.stop).start()
         self.state = MediaState.Stopped
 
     def volume(self, display, vol):

openlp/plugins/images/lib/mediaitem.py

@@ -353,7 +353,7 @@ class ImageMediaItem(MediaManagerItem):
                     icon = build_icon(thumb)
                 else:
                     icon = create_thumb(imageFile.filename, thumb)
-            item_name = QtGui.QTreeWidgetItem(filename)
+            item_name = QtGui.QTreeWidgetItem([filename])
             item_name.setText(0, filename)
             item_name.setIcon(0, icon)
             item_name.setToolTip(0, imageFile.filename)

tests/functional/openlp_core_ui/test_firsttimeform.py

@@ -31,12 +31,12 @@ Package to test the openlp.core.ui.firsttimeform package.
 """
 from unittest import TestCase
 
-from tests.functional import MagicMock
-
-from tests.helpers.testmixin import TestMixin
 from openlp.core.common import Registry
 from openlp.core.ui.firsttimeform import FirstTimeForm
 
+from tests.functional import MagicMock
+from tests.helpers.testmixin import TestMixin
+
 
 class TestFirstTimeForm(TestCase, TestMixin):
 

tests/functional/openlp_core_ui/test_mainwindow.py

@@ -34,6 +34,7 @@ import os
 from unittest import TestCase
 
 from openlp.core.ui.mainwindow import MainWindow
+from openlp.core.lib.ui import UiStrings
 from openlp.core.common.registry import Registry
 from tests.utils.constants import TEST_RESOURCES_PATH
 from tests.helpers.testmixin import TestMixin
@@ -95,3 +96,41 @@ class TestMainWindow(TestCase, TestMixin):
 
             # THEN the file should not be opened
             assert not mocked_load_path.called, 'load_path should not have been called'
+
+    def main_window_title_test(self):
+        """
+        Test that running a new instance of OpenLP set the window title correctly
+        """
+        # GIVEN a newly opened OpenLP instance
+
+        # WHEN no changes are made to the service
+
+        # THEN the main window's title shoud be the same as the OLPV2x string in the UiStrings class
+        self.assertEqual(self.main_window.windowTitle(), UiStrings().OLPV2x,
+                         'The main window\'s title should be the same as the OLPV2x string in UiStrings class')
+
+    def set_service_modifed_test(self):
+        """
+        Test that when setting the service's title the main window's title is set correctly
+        """
+        # GIVEN a newly opened OpenLP instance
+
+        # WHEN set_service_modified is called with with the modified flag set true and a file name
+        self.main_window.set_service_modified(True, 'test.osz')
+
+        # THEN the main window's title should be set to the
+        self.assertEqual(self.main_window.windowTitle(), '%s - %s*' % (UiStrings().OLPV2x, 'test.osz'),
+                         'The main window\'s title should be set to "<the contents of UiStrings().OLPV2x> - test.osz*"')
+
+    def set_service_unmodified_test(self):
+        """
+        Test that when setting the service's title the main window's title is set correctly
+        """
+        # GIVEN a newly opened OpenLP instance
+
+        # WHEN set_service_modified is called with with the modified flag set False and a file name
+        self.main_window.set_service_modified(False, 'test.osz')
+
+        # THEN the main window's title should be set to the
+        self.assertEqual(self.main_window.windowTitle(), '%s - %s' % (UiStrings().OLPV2x, 'test.osz'),
+                         'The main window\'s title should be set to "<the contents of UiStrings().OLPV2x> - test.osz"')