Phill has proposed merging lp:~phill-ridout/openlp/bug1412784 into lp:openlp.
Requested reviews: OpenLP Core (openlp-core) Related bugs: Bug #1412784 in OpenLP: "Preview media slider repeatedly shows "Error Occurred" dialogue box " https://bugs.launchpad.net/openlp/+bug/1412784 For more details, see: https://code.launchpad.net/~phill-ridout/openlp/bug1412784/+merge/247655 Fixes Bug #1412784: Preview media slider repeatedly shows "Error Occurred" dialogue box. The root cause of the problem is that webkit.check_available was returning true on windows even when the webkit player was not available. To fix this i've used "feature detection" to check that the html5 video tag is available. -------------------------------- lp:~phill-ridout/openlp/bug1412784 (revision 2496) [SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/910/ [SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/841/ [SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/787/ [SUCCESS] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/698/ [SUCCESS] http://ci.openlp.org/job/Branch-04b-Windows_Interface_Tests/297/ [SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/439/ [SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/310/ -- Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/bug1412784 into lp:openlp.
=== modified file 'openlp/core/ui/media/webkitplayer.py' --- openlp/core/ui/media/webkitplayer.py 2015-01-18 13:39:21 +0000 +++ openlp/core/ui/media/webkitplayer.py 2015-01-26 21:22:07 +0000 @@ -22,11 +22,11 @@ """ The :mod:`~openlp.core.ui.media.webkit` module contains our WebKit video player """ -from PyQt4 import QtGui +from PyQt4 import QtGui, QtWebKit import logging -from openlp.core.common import Settings, is_macosx +from openlp.core.common import Settings from openlp.core.lib import translate from openlp.core.ui.media import MediaState from openlp.core.ui.media.mediaplayer import MediaPlayer @@ -222,13 +222,15 @@ def check_available(self): """ - Check the availability of the media player + Check the availability of the media player. + + :return: boolean. True if available """ - # At the moment we don't have support for webkitplayer on Mac OS X - if is_macosx(): - return False - else: - return True + web = QtWebKit.QWebPage() + # This script should return '[object HTMLVideoElement]' if the html5 video is available in webkit. Otherwise it + # should return '[object HTMLUnknownElement]' + return web.mainFrame().evaluateJavaScript( + "Object.prototype.toString.call(document.createElement('video'));") == '[object HTMLVideoElement]' def load(self, display): """ === modified file 'openlp/plugins/bibles/lib/__init__.py' --- openlp/plugins/bibles/lib/__init__.py 2015-01-22 13:19:10 +0000 +++ openlp/plugins/bibles/lib/__init__.py 2015-01-26 21:22:07 +0000 @@ -178,7 +178,7 @@ default_separators = [ '|'.join([ translate('BiblesPlugin', ':', 'Verse identifier e.g. Genesis 1 : 1 = Genesis Chapter 1 Verse 1'), - translate('BiblesPlugin', 'v','Verse identifier e.g. Genesis 1 v 1 = Genesis Chapter 1 Verse 1'), + translate('BiblesPlugin', 'v', 'Verse identifier e.g. Genesis 1 v 1 = Genesis Chapter 1 Verse 1'), translate('BiblesPlugin', 'V', 'Verse identifier e.g. Genesis 1 V 1 = Genesis Chapter 1 Verse 1'), translate('BiblesPlugin', 'verse', 'Verse identifier e.g. Genesis 1 verse 1 = Genesis Chapter 1 Verse 1'), translate('BiblesPlugin', 'verses', === modified file 'tests/functional/openlp_core_ui_media/test_webkitplayer.py' --- tests/functional/openlp_core_ui_media/test_webkitplayer.py 2015-01-18 13:39:21 +0000 +++ tests/functional/openlp_core_ui_media/test_webkitplayer.py 2015-01-26 21:22:07 +0000 @@ -23,7 +23,7 @@ Package to test the openlp.core.ui.media.webkitplayer package. """ from unittest import TestCase -from tests.functional import patch +from tests.functional import MagicMock, patch from openlp.core.ui.media.webkitplayer import WebkitPlayer @@ -33,32 +33,36 @@ Test the functions in the :mod:`webkitplayer` module. """ - def check_available_mac_test(self): - """ - Simple test of webkitplayer availability on Mac OS X - """ - # GIVEN: A WebkitPlayer and a mocked is_macosx - with patch('openlp.core.ui.media.webkitplayer.is_macosx') as mocked_is_macosx: - mocked_is_macosx.return_value = True - webkit_player = WebkitPlayer(None) - - # WHEN: An checking if the player is available - available = webkit_player.check_available() - - # THEN: The player should not be available on Mac OS X - self.assertEqual(False, available, 'The WebkitPlayer should not be available on Mac OS X.') - - def check_available_non_mac_test(self): - """ - Simple test of webkitplayer availability when not on Mac OS X - """ - # GIVEN: A WebkitPlayer and a mocked is_macosx - with patch('openlp.core.ui.media.webkitplayer.is_macosx') as mocked_is_macosx: - mocked_is_macosx.return_value = False - webkit_player = WebkitPlayer(None) - - # WHEN: An checking if the player is available - available = webkit_player.check_available() - - # THEN: The player should be available when not on Mac OS X - self.assertEqual(True, available, 'The WebkitPlayer should be available when not on Mac OS X.') + def check_available_video_disabled_test(self): + """ + Test of webkit video unavailability + """ + # GIVEN: A WebkitPlayer instance and a mocked QWebPage + mocked_qwebpage = MagicMock() + mocked_qwebpage.mainFrame().evaluateJavaScript.return_value = '[object HTMLUnknownElement]' + with patch('openlp.core.ui.media.webkitplayer.QtWebKit.QWebPage', **{'return_value': mocked_qwebpage}): + webkit_player = WebkitPlayer(None) + + # WHEN: An checking if the player is available + available = webkit_player.check_available() + + # THEN: The player should not be available when '[object HTMLUnknownElement]' is returned + self.assertEqual(False, available, + 'The WebkitPlayer should not be available when video feature detection fails') + + def check_available_video_enabled_test(self): + """ + Test of webkit video availability + """ + # GIVEN: A WebkitPlayer instance and a mocked QWebPage + mocked_qwebpage = MagicMock() + mocked_qwebpage.mainFrame().evaluateJavaScript.return_value = '[object HTMLVideoElement]' + with patch('openlp.core.ui.media.webkitplayer.QtWebKit.QWebPage', **{'return_value': mocked_qwebpage}): + webkit_player = WebkitPlayer(None) + + # WHEN: An checking if the player is available + available = webkit_player.check_available() + + # THEN: The player should be available when '[object HTMLVideoElement]' is returned + self.assertEqual(True, available, + 'The WebkitPlayer should be available when video feature detection passes') === modified file 'tests/functional/test_init.py' --- tests/functional/test_init.py 2015-01-24 10:21:13 +0000 +++ tests/functional/test_init.py 2015-01-26 21:22:07 +0000 @@ -24,6 +24,7 @@ """ from optparse import Values import os +import sys from unittest import TestCase from unittest.mock import MagicMock, patch @@ -118,12 +119,27 @@ """ Test that parse_options parses short options correctly """ - # GIVEN: A list of vaild short options + # GIVEN: A list of valid short options options = ['-e', '-l', 'debug', '-pd', '-s', 'style', 'extra', 'qt', 'args'] # WHEN: Calling parse_options - resluts = parse_options(options) + results = parse_options(options) # THEN: A tuple should be returned with the parsed options and left over args - self.assertEqual(resluts, (Values({'no_error_form': True, 'dev_version': True, 'portable': True, + self.assertEqual(results, (Values({'no_error_form': True, 'dev_version': True, 'portable': True, + 'style': 'style', 'loglevel': 'debug'}), ['extra', 'qt', 'args'])) + + def parse_options_valid_argv_short_options_test(self): + """ + Test that parse_options parses valid short options correctly when passed through sys.argv + """ + # GIVEN: A list of valid options + options = ['openlp.py', '-e', '-l', 'debug', '-pd', '-s', 'style', 'extra', 'qt', 'args'] + + # WHEN: Passing in the options through sys.argv and calling parse_args with None + with patch.object(sys, 'argv', options): + results = parse_options(None) + + # THEN: parse_args should return a tuple of valid options and of left over options that OpenLP does not use + self.assertEqual(results, (Values({'no_error_form': True, 'dev_version': True, 'portable': True, 'style': 'style', 'loglevel': 'debug'}), ['extra', 'qt', 'args']))
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp