Merge lp:~tomasgroth/openlp/ppt-fixes into lp:openlp

[Tomas Groth]

Take focus back if Powerpoint steals it - fixes bug 1423913.
Optionally advance a Powerpoint slides animation when clicked in the slidecontroller - fixes bug 1194847.
Made OpenLP respect hidden slides. Improved logging in case of errors.
For Impress, go to previous effect instead of the previous slide.

[Tomas Groth (tomasgroth)]

lp:~tomasgroth/openlp/ppt-fixes (revision 2529)
[SUCCESS] https//ci.openlp.io/job/Branch-01-Pull/999/
[SUCCESS] https//ci.openlp.io/job/Branch-02-Functional-Tests/922/
[SUCCESS] https//ci.openlp.io/job/Branch-03-Interface-Tests/864/
[SUCCESS] https//ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/753/
[SUCCESS] https//ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/352/
[SUCCESS] https//ci.openlp.io/job/Branch-05a-Code_Analysis/487/
[SUCCESS] https//ci.openlp.io/job/Branch-05b-Test_Coverage/358/

[Tim Bentley (trb143)]

Looks OK

review: Approve

[Raoul Snyman (raoul-snyman)]

review: Approve

=== modified file 'openlp/plugins/presentations/lib/impresscontroller.py'
--- openlp/plugins/presentations/lib/impresscontroller.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/presentations/lib/impresscontroller.py	2015-04-02 08:41:26 +0000
@@ -428,7 +428,7 @@
         """
         Triggers the previous slide on the running presentation.
         """
-        self.control.gotoPreviousSlide()
+        self.control.gotoPreviousEffect()
 
     def get_slide_text(self, slide_no):
         """

=== modified file 'openlp/plugins/presentations/lib/messagelistener.py'
--- openlp/plugins/presentations/lib/messagelistener.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/presentations/lib/messagelistener.py	2015-04-02 08:41:26 +0000
@@ -93,7 +93,7 @@
             return True
         if not self.doc.is_loaded():
             if not self.doc.load_presentation():
-                log.warning('Failed to activate %s' % self.doc.filepath)
+                log.warning('Failed to activate %s' % self.doc.file_path)
                 return False
         if self.is_live:
             self.doc.start_presentation()
@@ -104,7 +104,7 @@
         if self.doc.is_active():
             return True
         else:
-            log.warning('Failed to activate %s' % self.doc.filepath)
+            log.warning('Failed to activate %s' % self.doc.file_path)
             return False
 
     def slide(self, slide):

=== modified file 'openlp/plugins/presentations/lib/powerpointcontroller.py'
--- openlp/plugins/presentations/lib/powerpointcontroller.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/presentations/lib/powerpointcontroller.py	2015-04-02 08:41:26 +0000
@@ -22,11 +22,14 @@
 """
 This module is for controlling powerpoint. PPT API documentation:
 `http://msdn.microsoft.com/en-us/library/aa269321(office.10).aspx`_
+2010: https://msdn.microsoft.com/en-us/library/office/ff743835%28v=office.14%29.aspx
+2013: https://msdn.microsoft.com/en-us/library/office/ff743835.aspx
 """
 import os
 import logging
+import time
 
-from openlp.core.common import is_win
+from openlp.core.common import is_win, Settings
 
 if is_win():
     from win32com.client import Dispatch
@@ -36,9 +39,8 @@
     import pywintypes
 
 from openlp.core.lib import ScreenList
-from openlp.core.common import Registry
 from openlp.core.lib.ui import UiStrings, critical_error_message_box, translate
-from openlp.core.common import trace_error_handler
+from openlp.core.common import trace_error_handler, Registry
 from .presentationcontroller import PresentationController, PresentationDocument
 
 log = logging.getLogger(__name__)
@@ -82,6 +84,7 @@
             if not self.process:
                 self.process = Dispatch('PowerPoint.Application')
             self.process.Visible = True
+            # ppWindowMinimized = 2
             self.process.WindowState = 2
 
         def kill(self):
@@ -97,8 +100,10 @@
                 if self.process.Presentations.Count > 0:
                     return
                 self.process.Quit()
-            except (AttributeError, pywintypes.com_error):
-                pass
+            except (AttributeError, pywintypes.com_error) as e:
+                log.exception('Exception caught while killing powerpoint process')
+                log.exception(e)
+                trace_error_handler(log)
             self.process = None
 
 
@@ -117,6 +122,8 @@
         log.debug('Init Presentation Powerpoint')
         super(PowerpointDocument, self).__init__(controller, presentation)
         self.presentation = None
+        self.index_map = {}
+        self.slide_count = 0
 
     def load_presentation(self):
         """
@@ -131,16 +138,21 @@
             self.presentation = self.controller.process.Presentations(self.controller.process.Presentations.Count)
             self.create_thumbnails()
             self.create_titles_and_notes()
-            # Powerpoint 2013 pops up when loading a file, so we minimize it again
-            if self.presentation.Application.Version == u'15.0':
+            # Powerpoint 2010 and 2013 pops up when loading a file, so we minimize it again
+            if float(self.presentation.Application.Version) >= 14.0:
                 try:
+                    # ppWindowMinimized = 2
                     self.presentation.Application.WindowState = 2
-                except:
-                    log.error('Failed to minimize main powerpoint window')
+                except (AttributeError, pywintypes.com_error) as e:
+                    log.exception('Failed to minimize main powerpoint window')
+                    log.exception(e)
                     trace_error_handler(log)
+            # Make sure powerpoint doesn't steal focus
+            Registry().get('main_window').activateWindow()
             return True
-        except pywintypes.com_error:
-            log.error('PPT open failed')
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Exception caught while loading Powerpoint presentation')
+            log.exception(e)
             trace_error_handler(log)
             return False
 
@@ -158,9 +170,14 @@
         log.debug('create_thumbnails')
         if self.check_thumbnails():
             return
+        key = 1
         for num in range(self.presentation.Slides.Count):
-            self.presentation.Slides(num + 1).Export(
-                os.path.join(self.get_thumbnail_folder(), 'slide%d.png' % (num + 1)), 'png', 320, 240)
+            if not self.presentation.Slides(num + 1).SlideShowTransition.Hidden:
+                self.index_map[key] = num + 1
+                self.presentation.Slides(num + 1).Export(
+                    os.path.join(self.get_thumbnail_folder(), 'slide%d.png' % (key)), 'png', 320, 240)
+                key += 1
+        self.slide_count = key - 1
 
     def close_presentation(self):
         """
@@ -171,10 +188,14 @@
         if self.presentation:
             try:
                 self.presentation.Close()
-            except pywintypes.com_error:
-                pass
+            except (AttributeError, pywintypes.com_error) as e:
+                log.exception('Caught exception while closing powerpoint presentation')
+                log.exception(e)
+                trace_error_handler(log)
         self.presentation = None
         self.controller.remove_doc(self)
+        # Make sure powerpoint doesn't steal focus
+        Registry().get('main_window').activateWindow()
 
     def is_loaded(self):
         """
@@ -188,7 +209,10 @@
                 return False
             if self.controller.process.Presentations.Count == 0:
                 return False
-        except (AttributeError, pywintypes.com_error):
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in is_loaded')
+            log.exception(e)
+            trace_error_handler(log)
             return False
         return True
 
@@ -204,7 +228,10 @@
                 return False
             if self.presentation.SlideShowWindow.View is None:
                 return False
-        except (AttributeError, pywintypes.com_error):
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in is_active')
+            log.exception(e)
+            trace_error_handler(log)
             return False
         return True
 
@@ -215,19 +242,21 @@
         log.debug('unblank_screen')
         try:
             self.presentation.SlideShowSettings.Run()
+            # ppSlideShowRunning = 1
             self.presentation.SlideShowWindow.View.State = 1
             self.presentation.SlideShowWindow.Activate()
-            if self.presentation.Application.Version == '14.0':
-                # Unblanking is broken in PowerPoint 2010, need to redisplay
-                slide = self.presentation.SlideShowWindow.View.CurrentShowPosition
-                click = self.presentation.SlideShowWindow.View.GetClickIndex()
-                self.presentation.SlideShowWindow.View.GotoSlide(slide)
-                if click:
-                    self.presentation.SlideShowWindow.View.GotoClick(click)
-        except pywintypes.com_error:
-            log.error('COM error while in unblank_screen')
+            # Unblanking is broken in PowerPoint 2010 and 2013, need to redisplay
+            if float(self.presentation.Application.Version) >= 14.0:
+                self.presentation.SlideShowWindow.View.GotoSlide(self.blank_slide, False)
+                if self.blank_click:
+                    self.presentation.SlideShowWindow.View.GotoClick(self.blank_click)
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in unblank_screen')
+            log.exception(e)
             trace_error_handler(log)
             self.show_error_msg()
+        # Make sure powerpoint doesn't steal focus
+        Registry().get('main_window').activateWindow()
 
     def blank_screen(self):
         """
@@ -235,9 +264,15 @@
         """
         log.debug('blank_screen')
         try:
+            # Unblanking is broken in PowerPoint 2010 and 2013, need to save info for later
+            if float(self.presentation.Application.Version) >= 14.0:
+                self.blank_slide = self.get_slide_number()
+                self.blank_click = self.presentation.SlideShowWindow.View.GetClickIndex()
+            # ppSlideShowBlackScreen = 3
             self.presentation.SlideShowWindow.View.State = 3
-        except pywintypes.com_error:
-            log.error('COM error while in blank_screen')
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in blank_screen')
+            log.exception(e)
             trace_error_handler(log)
             self.show_error_msg()
 
@@ -248,9 +283,11 @@
         log.debug('is_blank')
         if self.is_active():
             try:
+                # ppSlideShowBlackScreen = 3
                 return self.presentation.SlideShowWindow.View.State == 3
-            except pywintypes.com_error:
-                log.error('COM error while in is_blank')
+            except (AttributeError, pywintypes.com_error) as e:
+                log.exception('Caught exception while in is_blank')
+                log.exception(e)
                 trace_error_handler(log)
                 self.show_error_msg()
         else:
@@ -263,8 +300,9 @@
         log.debug('stop_presentation')
         try:
             self.presentation.SlideShowWindow.View.Exit()
-        except pywintypes.com_error:
-            log.error('COM error while in stop_presentation')
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in stop_presentation')
+            log.exception(e)
             trace_error_handler(log)
             self.show_error_msg()
 
@@ -292,15 +330,19 @@
                 ppt_window.Left = size.x() * 72 / dpi
                 ppt_window.Width = size.width() * 72 / dpi
             except AttributeError as e:
-                log.error('AttributeError while in start_presentation')
-                log.error(e)
-            # Powerpoint 2013 pops up when starting a file, so we minimize it again
-            if self.presentation.Application.Version == u'15.0':
+                log.exception('AttributeError while in start_presentation')
+                log.exception(e)
+            # Powerpoint 2010 and 2013 pops up when starting a file, so we minimize it again
+            if float(self.presentation.Application.Version) >= 14.0:
                 try:
+                    # ppWindowMinimized = 2
                     self.presentation.Application.WindowState = 2
-                except:
-                    log.error('Failed to minimize main powerpoint window')
+                except (AttributeError, pywintypes.com_error) as e:
+                    log.exception('Failed to minimize main powerpoint window')
+                    log.exception(e)
                     trace_error_handler(log)
+            # Make sure powerpoint doesn't steal focus
+            Registry().get('main_window').activateWindow()
 
     def get_slide_number(self):
         """
@@ -309,9 +351,19 @@
         log.debug('get_slide_number')
         ret = 0
         try:
-            ret = self.presentation.SlideShowWindow.View.CurrentShowPosition
-        except pywintypes.com_error:
-            log.error('COM error while in get_slide_number')
+            # We need 2 approaches to getting the current slide number, because
+            # SlideShowWindow.View.Slide.SlideIndex wont work on the end-slide where Slide isn't available, and
+            # SlideShowWindow.View.CurrentShowPosition returns 0 when called when a transistion is executing (in 2013)
+            # So we use SlideShowWindow.View.Slide.SlideIndex unless the state is done (ppSlideShowDone = 5)
+            if self.presentation.SlideShowWindow.View.State != 5:
+                ret = self.presentation.SlideShowWindow.View.Slide.SlideNumber
+                # Do reverse lookup in the index_map to find the slide number to return
+                ret = next((key for key, slidenum in self.index_map.items() if slidenum == ret), None)
+            else:
+                ret = self.presentation.SlideShowWindow.View.CurrentShowPosition
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in get_slide_number')
+            log.exception(e)
             trace_error_handler(log)
             self.show_error_msg()
         return ret
@@ -321,14 +373,7 @@
         Returns total number of slides.
         """
         log.debug('get_slide_count')
-        ret = 0
-        try:
-            ret = self.presentation.Slides.Count
-        except pywintypes.com_error:
-            log.error('COM error while in get_slide_count')
-            trace_error_handler(log)
-            self.show_error_msg()
-        return ret
+        return self.slide_count
 
     def goto_slide(self, slide_no):
         """
@@ -338,9 +383,19 @@
         """
         log.debug('goto_slide')
         try:
-            self.presentation.SlideShowWindow.View.GotoSlide(slide_no)
-        except pywintypes.com_error:
-            log.error('COM error while in goto_slide')
+            if Settings().value('presentations/powerpoint slide click advance') \
+                    and self.get_slide_number() == self.index_map[slide_no]:
+                click_index = self.presentation.SlideShowWindow.View.GetClickIndex()
+                click_count = self.presentation.SlideShowWindow.View.GetClickCount()
+                log.debug('We are already on this slide - go to next effect if any left, idx: %d, count: %d'
+                          % (click_index, click_count))
+                if click_index < click_count:
+                    self.next_step()
+            else:
+                self.presentation.SlideShowWindow.View.GotoSlide(self.index_map[slide_no])
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in goto_slide')
+            log.exception(e)
             trace_error_handler(log)
             self.show_error_msg()
 
@@ -351,12 +406,14 @@
         log.debug('next_step')
         try:
             self.presentation.SlideShowWindow.View.Next()
-        except pywintypes.com_error:
-            log.error('COM error while in next_step')
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in next_step')
+            log.exception(e)
             trace_error_handler(log)
             self.show_error_msg()
             return
         if self.get_slide_number() > self.get_slide_count():
+            log.debug('past end, stepping back to previous')
             self.previous_step()
 
     def previous_step(self):
@@ -366,8 +423,9 @@
         log.debug('previous_step')
         try:
             self.presentation.SlideShowWindow.View.Previous()
-        except pywintypes.com_error:
-            log.error('COM error while in previous_step')
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Caught exception while in previous_step')
+            log.exception(e)
             trace_error_handler(log)
             self.show_error_msg()
 
@@ -377,7 +435,7 @@
 
         :param slide_no: The slide the text is required for, starting at 1
         """
-        return _get_text_from_shapes(self.presentation.Slides(slide_no).Shapes)
+        return _get_text_from_shapes(self.presentation.Slides(self.index_map[slide_no]).Shapes)
 
     def get_slide_notes(self, slide_no):
         """
@@ -385,7 +443,7 @@
 
         :param slide_no: The slide the text is required for, starting at 1
         """
-        return _get_text_from_shapes(self.presentation.Slides(slide_no).NotesPage.Shapes)
+        return _get_text_from_shapes(self.presentation.Slides(self.index_map[slide_no]).NotesPage.Shapes)
 
     def create_titles_and_notes(self):
         """
@@ -396,7 +454,8 @@
         """
         titles = []
         notes = []
-        for slide in self.presentation.Slides:
+        for num in range(self.get_slide_count()):
+            slide = self.presentation.Slides(self.index_map[num + 1])
             try:
                 text = slide.Shapes.Title.TextFrame.TextRange.Text
             except Exception as e:
@@ -413,7 +472,11 @@
         """
         Stop presentation and display an error message.
         """
-        self.stop_presentation()
+        try:
+            self.presentation.SlideShowWindow.View.Exit()
+        except (AttributeError, pywintypes.com_error) as e:
+            log.exception('Failed to exit Powerpoint presentation after error')
+            log.exception(e)
         critical_error_message_box(UiStrings().Error, translate('PresentationPlugin.PowerpointDocument',
                                                                 'An error occurred in the Powerpoint integration '
                                                                 'and the presentation will be stopped. '

=== modified file 'openlp/plugins/presentations/lib/presentationtab.py'
--- openlp/plugins/presentations/lib/presentationtab.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/presentations/lib/presentationtab.py	2015-04-02 08:41:26 +0000
@@ -68,6 +68,15 @@
         self.override_app_check_box.setObjectName('override_app_check_box')
         self.advanced_layout.addWidget(self.override_app_check_box)
         self.left_layout.addWidget(self.advanced_group_box)
+        # PowerPoint
+        self.powerpoint_group_box = QtGui.QGroupBox(self.left_column)
+        self.powerpoint_group_box.setObjectName('powerpoint_group_box')
+        self.powerpoint_layout = QtGui.QVBoxLayout(self.powerpoint_group_box)
+        self.powerpoint_layout.setObjectName('powerpoint_layout')
+        self.ppt_slide_click_check_box = QtGui.QCheckBox(self.powerpoint_group_box)
+        self.powerpoint_group_box.setObjectName('ppt_slide_click_check_box')
+        self.powerpoint_layout.addWidget(self.ppt_slide_click_check_box)
+        self.left_layout.addWidget(self.powerpoint_group_box)
         # Pdf options
         self.pdf_group_box = QtGui.QGroupBox(self.left_column)
         self.pdf_group_box.setObjectName('pdf_group_box')
@@ -108,8 +117,12 @@
             self.set_controller_text(checkbox, controller)
         self.advanced_group_box.setTitle(UiStrings().Advanced)
         self.pdf_group_box.setTitle(translate('PresentationPlugin.PresentationTab', 'PDF options'))
+        self.powerpoint_group_box.setTitle(translate('PresentationPlugin.PresentationTab', 'PowerPoint options'))
         self.override_app_check_box.setText(
             translate('PresentationPlugin.PresentationTab', 'Allow presentation application to be overridden'))
+        self.ppt_slide_click_check_box.setText(
+            translate('PresentationPlugin.PresentationTab',
+                      'Clicking on a selected slide in the slidecontroller advances to next effect.'))
         self.pdf_program_check_box.setText(
             translate('PresentationPlugin.PresentationTab', 'Use given full path for mudraw or ghostscript binary:'))
 
@@ -123,11 +136,18 @@
         """
         Load the settings.
         """
+        powerpoint_available = False
         for key in self.controllers:
             controller = self.controllers[key]
             checkbox = self.presenter_check_boxes[controller.name]
             checkbox.setChecked(Settings().value(self.settings_section + '/' + controller.name))
+            if controller.name == 'Powerpoint' and controller.is_available():
+                powerpoint_available = True
         self.override_app_check_box.setChecked(Settings().value(self.settings_section + '/override app'))
+        # Load Powerpoint settings
+        self.ppt_slide_click_check_box.setChecked(Settings().value(self.settings_section +
+                                                                   '/powerpoint slide click advance'))
+        self.ppt_slide_click_check_box.setEnabled(powerpoint_available)
         # load pdf-program settings
         enable_pdf_program = Settings().value(self.settings_section + '/enable_pdf_program')
         self.pdf_program_check_box.setChecked(enable_pdf_program)
@@ -161,6 +181,11 @@
         if Settings().value(setting_key) != self.override_app_check_box.checkState():
             Settings().setValue(setting_key, self.override_app_check_box.checkState())
             changed = True
+        # Save powerpoint settings
+        setting_key = self.settings_section + '/powerpoint slide click advance'
+        if Settings().value(setting_key) != self.ppt_slide_click_check_box.checkState():
+            Settings().setValue(setting_key, self.ppt_slide_click_check_box.checkState())
+            changed = True
         # Save pdf-settings
         pdf_program = self.pdf_program_path.text()
         enable_pdf_program = self.pdf_program_check_box.checkState()

=== modified file 'openlp/plugins/presentations/presentationplugin.py'
--- openlp/plugins/presentations/presentationplugin.py	2015-03-10 22:00:32 +0000
+++ openlp/plugins/presentations/presentationplugin.py	2015-04-02 08:41:26 +0000
@@ -44,7 +44,8 @@
                         'presentations/Powerpoint Viewer': QtCore.Qt.Checked,
                         'presentations/Pdf': QtCore.Qt.Checked,
                         'presentations/presentations files': [],
-                        'presentations/thumbnail_scheme': ''
+                        'presentations/thumbnail_scheme': '',
+                        'presentations/powerpoint slide click advance': QtCore.Qt.Unchecked
                         }
 
 

=== modified file 'tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py'
--- tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py	2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_plugins/presentations/test_powerpointcontroller.py	2015-04-02 08:41:26 +0000
@@ -33,11 +33,15 @@
 
 from openlp.plugins.presentations.lib.powerpointcontroller import PowerpointController, PowerpointDocument,\
     _get_text_from_shapes
-from openlp.core.common import is_win
+from openlp.core.common import is_win, Settings
 
 if is_win():
     import pywintypes
 
+__default_settings__ = {
+    'presentations/powerpoint slide click advance': True
+}
+
 
 class TestPowerpointController(TestCase, TestMixin):
     """
@@ -104,6 +108,7 @@
         self.mock_presentation_document_get_temp_folder.return_value = 'temp folder'
         self.file_name = os.path.join(TEST_RESOURCES_PATH, 'presentations', 'test.pptx')
         self.real_controller = PowerpointController(self.mock_plugin)
+        Settings().extend_default_settings(__default_settings__)
 
     def tearDown(self):
         """
@@ -126,6 +131,7 @@
                 instance = PowerpointDocument(self.mock_controller, self.mock_presentation)
                 instance.presentation = MagicMock()
                 instance.presentation.SlideShowWindow.View.GotoSlide = MagicMock(side_effect=pywintypes.com_error('1'))
+                instance.index_map[42] = 42
 
                 # WHEN: Calling goto_slide which will throw an exception
                 instance.goto_slide(42)
@@ -227,3 +233,23 @@
 
         # THEN: it should not fail but return empty string
         self.assertEqual(result, '', 'result should be empty')
+
+    def goto_slide_test(self):
+        """
+        Test that goto_slide goes to next effect if the slide is already displayed
+        """
+        # GIVEN: A Document with mocked controller, presentation, and mocked functions get_slide_number and next_step
+        doc = PowerpointDocument(self.mock_controller, self.mock_presentation)
+        doc.presentation = MagicMock()
+        doc.presentation.SlideShowWindow.View.GetClickIndex.return_value = 1
+        doc.presentation.SlideShowWindow.View.GetClickCount.return_value = 2
+        doc.get_slide_number = MagicMock()
+        doc.get_slide_number.return_value = 1
+        doc.next_step = MagicMock()
+        doc.index_map[1] = 1
+
+        # WHEN: Calling goto_slide
+        doc.goto_slide(1)
+
+        # THEN: next_step() should be call to try to advance to the next effect.
+        self.assertTrue(doc.next_step.called, 'next_step() should have been called!')