FTW issues fixed by using threads the right way.

- Refactor FRW to show up faster by downloading config file after show
- Fix a long-standing UI bug in FRW by supplying title and subtitle to config page

bzr-revno: 2458

openlp/core/__init__.py

@@ -122,6 +122,9 @@ class OpenLP(OpenLPMixin, QtGui.QApplication):
             ftw.initialize(screens)
             if ftw.exec_() == QtGui.QDialog.Accepted:
                 Settings().setValue('core/has run wizard', True)
+            elif ftw.was_cancelled:
+                QtCore.QCoreApplication.exit()
+                sys.exit()
         # Correct stylesheet bugs
         application_stylesheet = ''
         if not Settings().value('advanced/alternate rows'):

openlp/core/ui/firsttimeform.py

@@ -31,7 +31,6 @@ This module contains the first time wizard.
 """
 import logging
 import os
-import sys
 import time
 import urllib.request
 import urllib.parse
@@ -51,30 +50,48 @@ from .firsttimewizard import UiFirstTimeWizard, FirstTimePage
 log = logging.getLogger(__name__)
 
 
-class ThemeScreenshotThread(QtCore.QThread):
+class ThemeScreenshotWorker(QtCore.QObject):
     """
-    This thread downloads the theme screenshots.
+    This thread downloads a theme's screenshot
     """
+    screenshot_downloaded = QtCore.pyqtSignal(str, str)
+    finished = QtCore.pyqtSignal()
+
+    def __init__(self, themes_url, title, filename, screenshot):
+        """
+        Set up the worker object
+        """
+        self.was_download_cancelled = False
+        self.themes_url = themes_url
+        self.title = title
+        self.filename = filename
+        self.screenshot = screenshot
+        super(ThemeScreenshotWorker, self).__init__()
+
     def run(self):
         """
         Overridden method to run the thread.
         """
-        themes = self.parent().config.get('themes', 'files')
+        if self.was_download_cancelled:
-        themes = themes.split(',')
+            return
-        config = self.parent().config
+        try:
-        for theme in themes:
+            urllib.request.urlretrieve('%s%s' % (self.themes_url, self.screenshot),
-            # Stop if the wizard has been cancelled.
+                                       os.path.join(gettempdir(), 'openlp', self.screenshot))
-            if self.parent().was_download_cancelled:
+            # Signal that the screenshot has been downloaded
-                return
+            self.screenshot_downloaded.emit(self.title, self.filename)
-            title = config.get('theme_%s' % theme, 'title')
+        except:
-            filename = config.get('theme_%s' % theme, 'filename')
+            log.exception('Unable to download screenshot')
-            screenshot = config.get('theme_%s' % theme, 'screenshot')
+        finally:
-            urllib.request.urlretrieve('%s%s' % (self.parent().themes_url, screenshot),
+            self.finished.emit()
-                                       os.path.join(gettempdir(), 'openlp', screenshot))
+
-            item = QtGui.QListWidgetItem(title, self.parent().themes_list_widget)
+    @QtCore.pyqtSlot(bool)
-            item.setData(QtCore.Qt.UserRole, filename)
+    def set_download_canceled(self, toggle):
-            item.setCheckState(QtCore.Qt.Unchecked)
+        """
-            item.setFlags(item.flags() | QtCore.Qt.ItemIsUserCheckable)
+        Externally set if the download was canceled
+
+        :param toggle: Set if the download was canceled or not
+        """
+        self.was_download_cancelled = toggle
 
 
 class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
@@ -88,22 +105,25 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
         Create and set up the first time wizard.
         """
         super(FirstTimeForm, self).__init__(parent)
+        self.web_access = True
+        self.web = ''
         self.setup_ui(self)
 
     def get_next_page_id(self):
         """
         Returns the id of the next FirstTimePage to go to based on enabled plugins
         """
-        # The songs plugin is enabled
         if FirstTimePage.Welcome < self.currentId() < FirstTimePage.Songs and self.songs_check_box.isChecked():
-            print('Go for songs! %r' % self.songs_check_box.isChecked())
+            # If the songs plugin is enabled then go to the songs page
             return FirstTimePage.Songs
-        # The Bibles plugin is enabled
         elif FirstTimePage.Welcome < self.currentId() < FirstTimePage.Bibles and self.bible_check_box.isChecked():
+            # Otherwise, if the Bibles plugin is enabled then go to the Bibles page
             return FirstTimePage.Bibles
         elif FirstTimePage.Welcome < self.currentId() < FirstTimePage.Themes:
+            # Otherwise, if the current page is somewhere between the Welcome and the Themes pages, go to the themes
             return FirstTimePage.Themes
         else:
+            # If all else fails, go to the next page
             return self.currentId() + 1
 
     def nextId(self):
@@ -111,18 +131,20 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
         Determine the next page in the Wizard to go to.
         """
         self.application.process_events()
-        if self.currentId() == FirstTimePage.Plugins:
+        if self.currentId() == FirstTimePage.Download:
             if not self.web_access:
                 return FirstTimePage.NoInternet
             else:
-                return self.get_next_page_id()
+                return FirstTimePage.Plugins
+        elif self.currentId() == FirstTimePage.Plugins:
+            return self.get_next_page_id()
         elif self.currentId() == FirstTimePage.Progress:
             return -1
         elif self.currentId() == FirstTimePage.NoInternet:
             return FirstTimePage.Progress
         elif self.currentId() == FirstTimePage.Themes:
             self.application.set_busy_cursor()
-            while not self.theme_screenshot_thread.isFinished():
+            while not all([thread.isFinished() for thread in self.theme_screenshot_threads]):
                 time.sleep(0.1)
                 self.application.process_events()
             # Build the screenshot icons, as this can not be done in the thread.
@@ -146,11 +168,20 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
         :param screens: The screens detected by OpenLP
         """
         self.screens = screens
+        self.was_cancelled = False
+        self.theme_screenshot_threads = []
+        self.theme_screenshot_workers = []
+        self.has_run_wizard = False
+
+    def _download_index(self):
+        """
+        Download the configuration file and kick off the theme screenshot download threads
+        """
         # check to see if we have web access
         self.web_access = False
-        self.web = 'http://openlp.org/files/frw/'
         self.config = ConfigParser()
         user_agent = 'OpenLP/' + Registry().get('application').applicationVersion()
+        self.application.process_events()
         web_config = get_web_page('%s%s' % (self.web, 'download.cfg'), header=('User-Agent', user_agent))
         if web_config:
             files = web_config.read()
@@ -165,24 +196,8 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
                 log.debug('A problem occured while parsing the downloaded config file')
                 trace_error_handler(log)
         self.update_screen_list_combo()
-        self.was_download_cancelled = False
+        self.application.process_events()
-        self.theme_screenshot_thread = None
-        self.has_run_wizard = False
         self.downloading = translate('OpenLP.FirstTimeWizard', 'Downloading %s...')
-        self.cancel_button.clicked.connect(self.on_cancel_button_clicked)
-        self.no_internet_finish_button.clicked.connect(self.on_no_internet_finish_button_clicked)
-        self.currentIdChanged.connect(self.on_current_id_changed)
-        Registry().register_function('config_screen_changed', self.update_screen_list_combo)
-
-    def set_defaults(self):
-        """
-        Set up display at start of theme edit.
-        """
-        self.restart()
-        check_directory_exists(os.path.join(gettempdir(), 'openlp'))
-        self.no_internet_finish_button.setVisible(False)
-        # Check if this is a re-run of the wizard.
-        self.has_run_wizard = Settings().value('core/has run wizard')
         if self.has_run_wizard:
             self.songs_check_box.setChecked(self.plugin_manager.get_plugin_by_name('songs').is_active())
             self.bible_check_box.setChecked(self.plugin_manager.get_plugin_by_name('bibles').is_active())
@@ -199,6 +214,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
             songs = self.config.get('songs', 'languages')
             songs = songs.split(',')
             for song in songs:
+                self.application.process_events()
                 title = self.config.get('songs_%s' % song, 'title')
                 filename = self.config.get('songs_%s' % song, 'filename')
                 item = QtGui.QListWidgetItem(title, self.songs_list_widget)
@@ -208,11 +224,13 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
             bible_languages = self.config.get('bibles', 'languages')
             bible_languages = bible_languages.split(',')
             for lang in bible_languages:
+                self.application.process_events()
                 language = self.config.get('bibles_%s' % lang, 'title')
                 lang_item = QtGui.QTreeWidgetItem(self.bibles_tree_widget, [language])
                 bibles = self.config.get('bibles_%s' % lang, 'translations')
                 bibles = bibles.split(',')
                 for bible in bibles:
+                    self.application.process_events()
                     title = self.config.get('bible_%s' % bible, 'title')
                     filename = self.config.get('bible_%s' % bible, 'filename')
                     item = QtGui.QTreeWidgetItem(lang_item, [title])
@@ -220,9 +238,38 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
                     item.setCheckState(0, QtCore.Qt.Unchecked)
                     item.setFlags(item.flags() | QtCore.Qt.ItemIsUserCheckable)
             self.bibles_tree_widget.expandAll()
-            # Download the theme screenshots.
+            self.application.process_events()
-            self.theme_screenshot_thread = ThemeScreenshotThread(self)
+            # Download the theme screenshots
-            self.theme_screenshot_thread.start()
+            themes = self.config.get('themes', 'files').split(',')
+            for theme in themes:
+                self.application.process_events()
+                title = self.config.get('theme_%s' % theme, 'title')
+                filename = self.config.get('theme_%s' % theme, 'filename')
+                screenshot = self.config.get('theme_%s' % theme, 'screenshot')
+                worker = ThemeScreenshotWorker(self.themes_url, title, filename, screenshot)
+                self.theme_screenshot_workers.append(worker)
+                worker.screenshot_downloaded.connect(self.on_screenshot_downloaded)
+                thread = QtCore.QThread(self)
+                self.theme_screenshot_threads.append(thread)
+                thread.started.connect(worker.run)
+                worker.finished.connect(thread.quit)
+                worker.moveToThread(thread)
+                thread.start()
+
+    def set_defaults(self):
+        """
+        Set up display at start of theme edit.
+        """
+        self.restart()
+        self.web = 'http://openlp.org/files/frw/'
+        self.cancel_button.clicked.connect(self.on_cancel_button_clicked)
+        self.no_internet_finish_button.clicked.connect(self.on_no_internet_finish_button_clicked)
+        self.currentIdChanged.connect(self.on_current_id_changed)
+        Registry().register_function('config_screen_changed', self.update_screen_list_combo)
+        self.no_internet_finish_button.setVisible(False)
+        # Check if this is a re-run of the wizard.
+        self.has_run_wizard = Settings().value('core/has run wizard')
+        check_directory_exists(os.path.join(gettempdir(), 'openlp'))
 
     def update_screen_list_combo(self):
         """
@@ -241,12 +288,20 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
         self.application.process_events()
         if page_id != -1:
             self.last_id = page_id
-        if page_id == FirstTimePage.Plugins:
+        if page_id == FirstTimePage.Download:
+            self.back_button.setVisible(False)
+            self.next_button.setVisible(False)
             # Set the no internet page text.
             if self.has_run_wizard:
                 self.no_internet_label.setText(self.no_internet_text)
             else:
                 self.no_internet_label.setText(self.no_internet_text + self.cancel_wizard_text)
+            self.application.set_busy_cursor()
+            self._download_index()
+            self.application.set_normal_cursor()
+            self.back_button.setVisible(False)
+            self.next_button.setVisible(True)
+            self.next()
         elif page_id == FirstTimePage.Defaults:
             self.theme_combo_box.clear()
             for index in range(self.themes_list_widget.count()):
@@ -266,15 +321,12 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
         elif page_id == FirstTimePage.NoInternet:
             self.back_button.setVisible(False)
             self.next_button.setVisible(False)
+            self.cancel_button.setVisible(False)
             self.no_internet_finish_button.setVisible(True)
-            if self.has_run_wizard:
+        elif page_id == FirstTimePage.Plugins:
-                self.cancel_button.setVisible(False)
+            self.back_button.setVisible(False)
         elif page_id == FirstTimePage.Progress:
             self.application.set_busy_cursor()
-            self.repaint()
-            self.application.process_events()
-            # Try to give the wizard a chance to redraw itself
-            time.sleep(0.2)
             self._pre_wizard()
             self._perform_wizard()
             self._post_wizard()
@@ -284,17 +336,28 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
         """
         Process the triggering of the cancel button.
         """
-        if self.last_id == FirstTimePage.NoInternet or \
+        self.was_cancelled = True
-                (self.last_id <= FirstTimePage.Plugins and not self.has_run_wizard):
+        if self.theme_screenshot_workers:
-            QtCore.QCoreApplication.exit()
+            for worker in self.theme_screenshot_workers:
-            sys.exit()
+                worker.set_download_canceled(True)
-        self.was_download_cancelled = True
         # Was the thread created.
-        if self.theme_screenshot_thread:
+        if self.theme_screenshot_threads:
-            while self.theme_screenshot_thread.isRunning():
+            while any([thread.isRunning() for thread in self.theme_screenshot_threads]):
                 time.sleep(0.1)
         self.application.set_normal_cursor()
 
+    def on_screenshot_downloaded(self, title, filename):
+        """
+        Add an item to the list when a theme has been downloaded
+
+        :param title: The title of the theme
+        :param filename: The filename of the theme
+        """
+        item = QtGui.QListWidgetItem(title, self.themes_list_widget)
+        item.setData(QtCore.Qt.UserRole, filename)
+        item.setCheckState(QtCore.Qt.Unchecked)
+        item.setFlags(item.flags() | QtCore.Qt.ItemIsUserCheckable)
+
     def on_no_internet_finish_button_clicked(self):
         """
         Process the triggering of the "Finish" button on the No Internet page.
@@ -321,7 +384,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
                 url_file = urllib.request.urlopen(url, timeout=CONNECTION_TIMEOUT)
                 filename = open(f_path, "wb")
                 # Download until finished or canceled.
-                while not self.was_download_cancelled:
+                while not self.was_cancelled:
                     data = url_file.read(block_size)
                     if not data:
                         break
@@ -341,7 +404,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
                     continue
             break
         # Delete file if cancelled, it may be a partial file.
-        if self.was_download_cancelled:
+        if self.was_cancelled:
             os.remove(f_path)
         return True
 
@@ -354,6 +417,7 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties):
         for index, theme in enumerate(themes):
             screenshot = self.config.get('theme_%s' % theme, 'screenshot')
             item = self.themes_list_widget.item(index)
+            # if item:
             item.setIcon(build_icon(os.path.join(gettempdir(), 'openlp', screenshot)))
 
     def _get_file_size(self, url):

openlp/core/ui/firsttimewizard.py

@@ -41,13 +41,14 @@ class FirstTimePage(object):
     An enumeration class with each of the pages of the wizard.
     """
     Welcome = 0
-    Plugins = 1
+    Download = 1
     NoInternet = 2
-    Songs = 3
+    Plugins = 3
-    Bibles = 4
+    Songs = 4
-    Themes = 5
+    Bibles = 5
-    Defaults = 6
+    Themes = 6
-    Progress = 7
+    Defaults = 7
+    Progress = 8
 
 
 class UiFirstTimeWizard(object):
@@ -78,6 +79,27 @@ class UiFirstTimeWizard(object):
         self.next_button = self.button(QtGui.QWizard.NextButton)
         self.back_button = self.button(QtGui.QWizard.BackButton)
         add_welcome_page(first_time_wizard, ':/wizards/wizard_firsttime.bmp')
+        # The download page
+        self.download_page = QtGui.QWizardPage()
+        self.download_page.setObjectName('download_page')
+        self.download_layout = QtGui.QVBoxLayout(self.download_page)
+        self.download_layout.setMargin(48)
+        self.download_layout.setObjectName('download_layout')
+        self.download_label = QtGui.QLabel(self.download_page)
+        self.download_label.setObjectName('download_label')
+        self.download_layout.addWidget(self.download_label)
+        first_time_wizard.setPage(FirstTimePage.Download, self.download_page)
+        # The "you don't have an internet connection" page.
+        self.no_internet_page = QtGui.QWizardPage()
+        self.no_internet_page.setObjectName('no_internet_page')
+        self.no_internet_layout = QtGui.QVBoxLayout(self.no_internet_page)
+        self.no_internet_layout.setContentsMargins(50, 30, 50, 40)
+        self.no_internet_layout.setObjectName('no_internet_layout')
+        self.no_internet_label = QtGui.QLabel(self.no_internet_page)
+        self.no_internet_label.setWordWrap(True)
+        self.no_internet_label.setObjectName('no_internet_label')
+        self.no_internet_layout.addWidget(self.no_internet_label)
+        first_time_wizard.setPage(FirstTimePage.NoInternet, self.no_internet_page)
         # The plugins page
         self.plugin_page = QtGui.QWizardPage()
         self.plugin_page.setObjectName('plugin_page')
@@ -120,17 +142,6 @@ class UiFirstTimeWizard(object):
         self.alert_check_box.setObjectName('alert_check_box')
         self.plugin_layout.addWidget(self.alert_check_box)
         first_time_wizard.setPage(FirstTimePage.Plugins, self.plugin_page)
-        # The "you don't have an internet connection" page.
-        self.no_internet_page = QtGui.QWizardPage()
-        self.no_internet_page.setObjectName('no_internet_page')
-        self.no_internet_layout = QtGui.QVBoxLayout(self.no_internet_page)
-        self.no_internet_layout.setContentsMargins(50, 30, 50, 40)
-        self.no_internet_layout.setObjectName('no_internet_layout')
-        self.no_internet_label = QtGui.QLabel(self.no_internet_page)
-        self.no_internet_label.setWordWrap(True)
-        self.no_internet_label.setObjectName('no_internet_label')
-        self.no_internet_layout.addWidget(self.no_internet_label)
-        first_time_wizard.setPage(FirstTimePage.NoInternet, self.no_internet_page)
         # The song samples page
         self.songs_page = QtGui.QWizardPage()
         self.songs_page.setObjectName('songs_page')
@@ -221,6 +232,11 @@ class UiFirstTimeWizard(object):
             translate('OpenLP.FirstTimeWizard', 'This wizard will help you to configure OpenLP for initial use. '
                                                 'Click the %s button below to start.') %
             clean_button_text(first_time_wizard.buttonText(QtGui.QWizard.NextButton)))
+        self.download_page.setTitle(translate('OpenLP.FirstTimeWizard', 'Downloading Resource Index'))
+        self.download_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Please wait while the resource index is '
+                                                                           'downloaded.'))
+        self.download_label.setText(translate('OpenLP.FirstTimeWizard', 'Please wait while OpenLP downloads the '
+                                                                        'resource index file...'))
         self.plugin_page.setTitle(translate('OpenLP.FirstTimeWizard', 'Activate required Plugins'))
         self.plugin_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Select the Plugins you wish to use. '))
         self.songs_check_box.setText(translate('OpenLP.FirstTimeWizard', 'Songs'))
@@ -257,5 +273,8 @@ class UiFirstTimeWizard(object):
                                                  'Set up default settings to be used by OpenLP.'))
         self.display_label.setText(translate('OpenLP.FirstTimeWizard', 'Default output display:'))
         self.theme_label.setText(translate('OpenLP.FirstTimeWizard', 'Select default theme:'))
+        self.progress_page.setTitle(translate('OpenLP.FirstTimeWizard', 'Downloading and Configuring'))
+        self.progress_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Please wait while resources are downloaded '
+                                                                           'and OpenLP is configured.'))
         self.progress_label.setText(translate('OpenLP.FirstTimeWizard', 'Starting configuration process...'))
         first_time_wizard.setButtonText(QtGui.QWizard.CustomButton1, translate('OpenLP.FirstTimeWizard', 'Finish'))

openlp/core/ui/mainwindow.py

@@ -690,7 +690,7 @@ class MainWindow(QtGui.QMainWindow, Ui_MainWindow, RegistryProperties):
         first_run_wizard = FirstTimeForm(self)
         first_run_wizard.initialize(ScreenList())
         first_run_wizard.exec_()
-        if first_run_wizard.was_download_cancelled:
+        if first_run_wizard.was_cancelled:
             return
         self.application.set_busy_cursor()
         self.first_time()

tests/functional/openlp_core_ui/test_firsttimeform.py

@@ -30,6 +30,7 @@
 Package to test the openlp.core.ui.firsttimeform package.
 """
 from configparser import ConfigParser
+import os
 from unittest import TestCase
 
 from openlp.core.common import Registry
@@ -71,55 +72,117 @@ class TestFirstTimeForm(TestCase, TestMixin):
     def setUp(self):
         self.setup_application()
         self.app.setApplicationVersion('0.0')
+        # Set up a fake "set_normal_cursor" method since we're not dealing with an actual OpenLP application object
+        self.app.set_normal_cursor = lambda: None
+        self.app.process_events = lambda: None
         Registry.create()
         Registry().register('application', self.app)
 
-    def basic_initialise_test(self):
+    def initialise_test(self):
         """
-        Test if we can intialise the FirstTimeForm without a config file
+        Test if we can intialise the FirstTimeForm
         """
-        # GIVEN: A mocked get_web_page, a First Time Wizard and an expected screen object
+        # GIVEN: A First Time Wizard and an expected screen object
-        with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
+        frw = FirstTimeForm(None)
-            first_time_form = FirstTimeForm(None)
+        expected_screens = MagicMock()
-            expected_screens = MagicMock()
-            expected_web_url = 'http://openlp.org/files/frw/'
-            expected_user_agent = 'OpenLP/0.0'
-            mocked_get_web_page.return_value = None
 
-            # WHEN: The First Time Wizard is initialised
+        # WHEN: The First Time Wizard is initialised
-            first_time_form.initialize(expected_screens)
+        frw.initialize(expected_screens)
 
-            # THEN: The First Time Form web configuration file should be accessible and parseable
+        # THEN: The screens should be set up, and the default values initialised
-            self.assertEqual(expected_screens, first_time_form.screens, 'The screens should be correct')
+        self.assertEqual(expected_screens, frw.screens, 'The screens should be correct')
-            self.assertEqual(expected_web_url, first_time_form.web, 'The base path of the URL should be correct')
+        self.assertTrue(frw.web_access, 'The default value of self.web_access should be True')
-            self.assertIsInstance(first_time_form.config, ConfigParser, 'The config object should be a ConfigParser')
+        self.assertFalse(frw.was_cancelled, 'The default value of self.was_cancelled should be False')
-            mocked_get_web_page.assert_called_with(expected_web_url + 'download.cfg',
+        self.assertListEqual([], frw.theme_screenshot_threads, 'The list of threads should be empty')
-                                                   header=('User-Agent', expected_user_agent))
+        self.assertListEqual([], frw.theme_screenshot_workers, 'The list of workers should be empty')
+        self.assertFalse(frw.has_run_wizard, 'has_run_wizard should be False')
 
-    def config_initialise_test(self):
+    def set_defaults_test(self):
         """
-        Test if we can intialise the FirstTimeForm with a config file
+        Test that the default values are set when set_defaults() is run
         """
-        # GIVEN: A mocked get_web_page, a First Time Wizard and an expected screen object
+        # GIVEN: An initialised FRW and a whole lot of stuff mocked out
-        with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
+        frw = FirstTimeForm(None)
-            first_time_form = FirstTimeForm(None)
+        frw.initialize(MagicMock())
-            expected_web_url = 'http://openlp.org/files/frw/'
+        with patch.object(frw, 'restart') as mocked_restart, \
-            expected_songs_url = 'http://example.com/frw/songs/'
+                patch.object(frw, 'cancel_button') as mocked_cancel_button, \
-            expected_bibles_url = 'http://example.com/frw/bibles/'
+                patch.object(frw, 'no_internet_finish_button') as mocked_no_internet_finish_btn, \
-            expected_themes_url = 'http://example.com/frw/themes/'
+                patch.object(frw, 'currentIdChanged') as mocked_currentIdChanged, \
-            expected_user_agent = 'OpenLP/0.0'
+                patch.object(Registry, 'register_function') as mocked_register_function, \
-            mocked_get_web_page.return_value.read.return_value = FAKE_CONFIG
+                patch('openlp.core.ui.firsttimeform.Settings') as MockedSettings, \
+                patch('openlp.core.ui.firsttimeform.gettempdir') as mocked_gettempdir, \
+                patch('openlp.core.ui.firsttimeform.check_directory_exists') as mocked_check_directory_exists, \
+                patch.object(frw.application, 'set_normal_cursor') as mocked_set_normal_cursor:
+            mocked_settings = MagicMock()
+            mocked_settings.value.return_value = True
+            MockedSettings.return_value = mocked_settings
+            mocked_gettempdir.return_value = 'temp'
+            expected_temp_path = os.path.join('temp', 'openlp')
 
-            # WHEN: The First Time Wizard is initialised
+            # WHEN: The set_defaults() method is run
-            first_time_form.initialize(MagicMock())
+            frw.set_defaults()
 
-            # THEN: The First Time Form web configuration file should be accessible and parseable
+            # THEN: The default values should have been set
-            self.assertIsInstance(first_time_form.config, ConfigParser, 'The config object should be a ConfigParser')
+            mocked_restart.assert_called_with()
-            mocked_get_web_page.assert_called_with(expected_web_url + 'download.cfg',
+            self.assertEqual('http://openlp.org/files/frw/', frw.web, 'The default URL should be set')
-                                                   header=('User-Agent', expected_user_agent))
+            mocked_cancel_button.clicked.connect.assert_called_with(frw.on_cancel_button_clicked)
-            self.assertEqual(expected_songs_url, first_time_form.songs_url, 'The songs URL should be correct')
+            mocked_no_internet_finish_btn.clicked.connect.assert_called_with(frw.on_no_internet_finish_button_clicked)
-            self.assertEqual(expected_bibles_url, first_time_form.bibles_url, 'The bibles URL should be correct')
+            mocked_currentIdChanged.connect.assert_called_with(frw.on_current_id_changed)
-            self.assertEqual(expected_themes_url, first_time_form.themes_url, 'The themes URL should be correct')
+            mocked_register_function.assert_called_with('config_screen_changed', frw.update_screen_list_combo)
+            mocked_no_internet_finish_btn.setVisible.assert_called_with(False)
+            mocked_settings.value.assert_called_with('core/has run wizard')
+            mocked_gettempdir.assert_called_with()
+            mocked_check_directory_exists.assert_called_with(expected_temp_path)
+
+    def update_screen_list_combo_test(self):
+        """
+        Test that the update_screen_list_combo() method works correctly
+        """
+        # GIVEN: A mocked Screen() object and an initialised First Run Wizard and a mocked display_combo_box
+        expected_screen_list = ['Screen 1', 'Screen 2']
+        mocked_screens = MagicMock()
+        mocked_screens.get_screen_list.return_value = expected_screen_list
+        frw = FirstTimeForm(None)
+        frw.initialize(mocked_screens)
+        with patch.object(frw, 'display_combo_box') as mocked_display_combo_box:
+            mocked_display_combo_box.count.return_value = 2
+
+            # WHEN: update_screen_list_combo() is called
+            frw.update_screen_list_combo()
+
+            # THEN: The combobox should have been updated
+            mocked_display_combo_box.clear.assert_called_with()
+            mocked_screens.get_screen_list.assert_called_with()
+            mocked_display_combo_box.addItems.assert_called_with(expected_screen_list)
+            mocked_display_combo_box.count.assert_called_with()
+            mocked_display_combo_box.setCurrentIndex.assert_called_with(1)
+
+    def on_cancel_button_clicked_test(self):
+        """
+        Test that the cancel button click slot shuts down the threads correctly
+        """
+        # GIVEN: A FRW, some mocked threads and workers (that isn't quite done) and other mocked stuff
+        frw = FirstTimeForm(None)
+        frw.initialize(MagicMock())
+        mocked_worker = MagicMock()
+        mocked_thread = MagicMock()
+        mocked_thread.isRunning.side_effect = [True, False]
+        frw.theme_screenshot_workers.append(mocked_worker)
+        frw.theme_screenshot_threads.append(mocked_thread)
+        with patch('openlp.core.ui.firsttimeform.time') as mocked_time, \
+                patch.object(frw.application, 'set_normal_cursor') as mocked_set_normal_cursor:
+
+            # WHEN: on_cancel_button_clicked() is called
+            frw.on_cancel_button_clicked()
+
+            # THEN: The right things should be called in the right order
+            self.assertTrue(frw.was_cancelled, 'The was_cancelled property should have been set to True')
+            mocked_worker.set_download_canceled.assert_called_with(True)
+            mocked_thread.isRunning.assert_called_with()
+            self.assertEqual(2, mocked_thread.isRunning.call_count, 'isRunning() should have been called twice')
+            mocked_time.sleep.assert_called_with(0.1)
+            self.assertEqual(1, mocked_time.sleep.call_count, 'sleep() should have only been called once')
+            mocked_set_normal_cursor.assert_called_with()
 
     def broken_config_test(self):
         """
@@ -128,10 +191,11 @@ class TestFirstTimeForm(TestCase, TestMixin):
         # GIVEN: A mocked get_web_page, a First Time Wizard, an expected screen object, and a mocked broken config file
         with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
             first_time_form = FirstTimeForm(None)
+            first_time_form.initialize(MagicMock())
             mocked_get_web_page.return_value.read.return_value = FAKE_BROKEN_CONFIG
 
-            # WHEN: The First Time Wizard is initialised
+            # WHEN: The First Time Wizard is downloads the config file
-            first_time_form.initialize(MagicMock())
+            first_time_form._download_index()
 
             # THEN: The First Time Form should not have web access
             self.assertFalse(first_time_form.web_access, 'There should not be web access with a broken config file')
@@ -143,10 +207,11 @@ class TestFirstTimeForm(TestCase, TestMixin):
         # GIVEN: A mocked get_web_page, a First Time Wizard, an expected screen object, and a mocked invalid config file
         with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
             first_time_form = FirstTimeForm(None)
+            first_time_form.initialize(MagicMock())
             mocked_get_web_page.return_value.read.return_value = FAKE_INVALID_CONFIG
 
-            # WHEN: The First Time Wizard is initialised
+            # WHEN: The First Time Wizard is downloads the config file
-            first_time_form.initialize(MagicMock())
+            first_time_form._download_index()
 
             # THEN: The First Time Form should not have web access
             self.assertFalse(first_time_form.web_access, 'There should not be web access with an invalid config file')