From b9b164866e9ddc79a21af2036bbb0fd1395b7406 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 27 Jun 2016 11:06:54 -0400 Subject: [PATCH 1/6] Update BlockStructure version number https://openedx.atlassian.net/browse/TNL-4865 Without this change, we get 500 errors when the server tries to deserialize cached data using a previous data structure. --- openedx/core/lib/block_structure/block_structure.py | 6 ++++++ openedx/core/lib/block_structure/cache.py | 7 +++++-- openedx/core/lib/block_structure/tests/test_manager.py | 7 +++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/openedx/core/lib/block_structure/block_structure.py b/openedx/core/lib/block_structure/block_structure.py index fea54061ad..00460bc2e7 100644 --- a/openedx/core/lib/block_structure/block_structure.py +++ b/openedx/core/lib/block_structure/block_structure.py @@ -395,6 +395,12 @@ class BlockStructureBlockData(BlockStructure): Subclass of BlockStructure that is responsible for managing block and transformer data. """ + # The latest version of the data structure of this class. Incrementally + # update this value whenever the data structure changes. Dependent storage + # layers can then use this value when serializing/deserializing block + # structures, and invalidating any previously cached/stored data. + VERSION = 1 + def __init__(self, root_block_usage_key): super(BlockStructureBlockData, self).__init__(root_block_usage_key) diff --git a/openedx/core/lib/block_structure/cache.py b/openedx/core/lib/block_structure/cache.py index 2ac1234086..3a02ed4c2e 100644 --- a/openedx/core/lib/block_structure/cache.py +++ b/openedx/core/lib/block_structure/cache.py @@ -6,7 +6,7 @@ from logging import getLogger from openedx.core.lib.cache_utils import zpickle, zunpickle -from .block_structure import BlockStructureModulestoreData +from .block_structure import BlockStructureModulestoreData, BlockStructureBlockData logger = getLogger(__name__) # pylint: disable=C0103 @@ -126,4 +126,7 @@ class BlockStructureCache(object): Returns the cache key to use for storing the block structure for the given root_block_usage_key. """ - return "root.key." + unicode(root_block_usage_key) + return "v{version}.root.key.{root_usage_key}".format( + version=unicode(BlockStructureBlockData.VERSION), + root_usage_key=unicode(root_block_usage_key), + ) diff --git a/openedx/core/lib/block_structure/tests/test_manager.py b/openedx/core/lib/block_structure/tests/test_manager.py index 5c32d6b08b..9ee7f762bc 100644 --- a/openedx/core/lib/block_structure/tests/test_manager.py +++ b/openedx/core/lib/block_structure/tests/test_manager.py @@ -4,6 +4,7 @@ Tests for manager.py from nose.plugins.attrib import attr from unittest import TestCase +from ..block_structure import BlockStructureBlockData from ..exceptions import UsageKeyNotInBlockStructure from ..manager import BlockStructureManager from ..transformers import BlockStructureTransformers @@ -154,6 +155,12 @@ class TestBlockStructureManager(TestCase, ChildrenMapTestMixin): self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) self.assertEquals(TestTransformer1.collect_call_count, 2) + def test_get_collected_version_update(self): + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) + BlockStructureBlockData.VERSION += 1 + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) + self.assertEquals(TestTransformer1.collect_call_count, 2) + def test_clear(self): self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) self.bs_manager.clear() From 0488f077dc341a7d483861aa61fb3aa8f92d6b89 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Fri, 24 Jun 2016 18:41:28 -0400 Subject: [PATCH 2/6] Fix discussion bundling in Studio --- cms/envs/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index a3204be72f..0d210366d3 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -620,7 +620,7 @@ PIPELINE_JS = { 'source_filenames': ( rooted_glob(COMMON_ROOT / 'static/', 'xmodule/descriptors/js/*.js') + rooted_glob(COMMON_ROOT / 'static/', 'xmodule/modules/js/*.js') + - rooted_glob(COMMON_ROOT / 'static/', 'coffee/src/discussion/*.js') + rooted_glob(COMMON_ROOT / 'static/', 'common/js/discussion/*.js') ), 'output_filename': 'js/cms-modules.js', 'test_order': 1 From 61d5fdcbd2f6104be1a2407dfac6a2ad8dd2ce00 Mon Sep 17 00:00:00 2001 From: Douglas Hall Date: Fri, 24 Jun 2016 15:15:55 -0400 Subject: [PATCH 3/6] Bump safe template linter thresholds to fix failing builds --- scripts/safelint_thresholds.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/safelint_thresholds.json b/scripts/safelint_thresholds.json index 0abc36af4b..ae45048842 100644 --- a/scripts/safelint_thresholds.json +++ b/scripts/safelint_thresholds.json @@ -1,13 +1,13 @@ { "rules": { - "javascript-concat-html": 205, + "javascript-concat-html": 213, "javascript-escape": 7, "javascript-interpolate": 49, - "javascript-jquery-append": 104, - "javascript-jquery-html": 275, + "javascript-jquery-append": 111, + "javascript-jquery-html": 279, "javascript-jquery-insert-into-target": 27, - "javascript-jquery-insertion": 26, - "javascript-jquery-prepend": 11, + "javascript-jquery-insertion": 29, + "javascript-jquery-prepend": 13, "mako-html-entities": 0, "mako-invalid-html-filter": 27, "mako-invalid-js-filter": 207, @@ -28,5 +28,5 @@ "python-wrap-html": 264, "underscore-not-escaped": 658 }, - "total": 2232 + "total": 2245 } From fba37a129d5399c850609a24cec592860e4cd8d1 Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Tue, 10 May 2016 14:58:02 -0400 Subject: [PATCH 4/6] AC-454 adding visual clarify for selected menu options --- .../xmodule/xmodule/css/video/display.scss | 2 + .../js/fixtures/video_yt_multiple.html | 9 +- .../xmodule/xmodule/js/karma_xmodule.conf.js | 9 +- .../xmodule/xmodule/js/spec/main_requirejs.js | 40 +++- .../js/spec/video/video_caption_spec.js | 4 +- .../js/spec/video/video_speed_control_spec.js | 14 +- .../js/src/video/08_video_speed_control.js | 80 +++++-- .../xmodule/js/src/video/09_video_caption.js | 206 ++++++++++++------ 8 files changed, 258 insertions(+), 106 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index bd478ab0a2..060842d788 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -535,6 +535,8 @@ .speed-option, .control-lang { + @include border-left($baseline/10 solid rgb(14, 166, 236)); + font-weight: $font-bold; color: rgb(14, 166, 236); // UXPL primary accent } } diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html index d4f5401785..c941c535a6 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html @@ -55,8 +55,9 @@ - -
+
+
+
@@ -108,7 +109,9 @@ -
+
+
+
diff --git a/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js b/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js index 5ee38962a7..3f72dcb7b0 100644 --- a/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js +++ b/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js @@ -20,12 +20,10 @@ var options = { libraryFilesToInclude: [ {pattern: 'common_static/js/vendor/requirejs/require.js', included: true}, {pattern: 'RequireJS-namespace-undefine.js', included: true}, - {pattern: 'spec/main_requirejs.js', included: true}, {pattern: 'common_static/coffee/src/ajax_prefix.js', included: true}, {pattern: 'common_static/common/js/vendor/underscore.js', included: true}, {pattern: 'common_static/common/js/vendor/backbone.js', included: true}, - {pattern: 'common_static/edx-ui-toolkit/js/utils/global-loader.js', included: true}, {pattern: 'common_static/js/vendor/CodeMirror/codemirror.js', included: true}, {pattern: 'common_static/js/vendor/draggabilly.js'}, {pattern: 'common_static/common/js/vendor/jquery.js', included: true}, @@ -50,11 +48,14 @@ var options = { {pattern: 'common_static/js/vendor/jasmine-imagediff.js', included: true}, {pattern: 'common_static/common/js/spec_helpers/jasmine-waituntil.js', included: true}, {pattern: 'common_static/common/js/spec_helpers/jasmine-extensions.js', included: true}, - {pattern: 'common_static/js/vendor/sinon-1.17.0.js', included: true} + {pattern: 'common_static/js/vendor/sinon-1.17.0.js', included: true}, + + {pattern: 'spec/main_requirejs.js', included: true}, ], libraryFiles: [ - {pattern: 'common_static/edx-pattern-library/js/**/*.js'} + {pattern: 'common_static/edx-pattern-library/js/**/*.js'}, + {pattern: 'common_static/edx-ui-toolkit/js/**/*.js'} ], // Make sure the patterns in sourceFiles and specFiles do not match the same file. diff --git a/common/lib/xmodule/xmodule/js/spec/main_requirejs.js b/common/lib/xmodule/xmodule/js/spec/main_requirejs.js index a57497f8c8..d4692469e5 100644 --- a/common/lib/xmodule/xmodule/js/spec/main_requirejs.js +++ b/common/lib/xmodule/xmodule/js/spec/main_requirejs.js @@ -1,4 +1,36 @@ -(function(requirejs) { +(function(requirejs, define) { + 'use strict'; + // We do not wish to bundle common libraries (that may also be used by non-RequireJS code on the page + // into the optimized files. Therefore load these libraries through script tags and explicitly define them. + // Note that when the optimizer executes this code, window will not be defined. + if (window) { + var defineDependency = function (globalName, name, noShim) { + var getGlobalValue = function(name) { + var globalNamePath = name.split('.'), + result = window, + i; + for (i = 0; i < globalNamePath.length; i++) { + result = result[globalNamePath[i]]; + } + return result; + }, + globalValue = getGlobalValue(globalName); + if (globalValue) { + if (noShim) { + define(name, {}); + } + else { + define(name, [], function() { return globalValue; }); + } + } + else { + console.error("Expected library to be included on page, but not found on window object: " + name); + } + }; + defineDependency("jQuery", "jquery"); + defineDependency("jQuery", "jquery-migrate"); + defineDependency("_", "underscore"); + } requirejs.config({ baseUrl: '/base/', paths: { @@ -6,7 +38,8 @@ "modernizr": "common_static/edx-pattern-library/js/modernizr-custom", "afontgarde": "common_static/edx-pattern-library/js/afontgarde", "edxicons": "common_static/edx-pattern-library/js/edx-icons", - "draggabilly": "common_static/js/vendor/draggabilly" + "draggabilly": "common_static/js/vendor/draggabilly", + 'edx-ui-toolkit': 'common_static/edx-ui-toolkit' }, "moment": { exports: "moment" @@ -18,5 +51,4 @@ exports: "AFontGarde" } }); - -}).call(this, RequireJS.requirejs); +}).call(this, RequireJS.requirejs, RequireJS.define); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js index b8cf3065b1..5d85be2551 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js @@ -266,6 +266,7 @@ expect($('.closed-captions')).toHaveAttrs({ 'lang': 'de' }); + expect(link).toHaveAttr('aria-pressed', 'true'); }); it('when clicking on link with current language', function () { @@ -284,6 +285,7 @@ expect(state.storage.setItem) .not.toHaveBeenCalledWith('language', 'en'); expect($('.langs-list li.is-active').length).toBe(1); + expect(link).toHaveAttr('aria-pressed', 'true'); }); it('open the language toggle on hover', function () { @@ -413,7 +415,7 @@ }); it('show explanation message', function () { - expect($('.subtitles-menu li')).toHaveText( + expect($('.subtitles .subtitles-menu li')).toHaveText( 'Transcript will be displayed when you start playing the video.' ); }); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js index 2a8b5e8b67..f73b1f3c59 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js @@ -203,16 +203,18 @@ describe('onSpeedChange', function () { beforeEach(function () { state = jasmine.initializePlayer(); - $('li[data-speed="1.0"]').addClass('is-active'); + $('li[data-speed="1.0"]').addClass('is-active').attr('aria-pressed', 'true'); state.videoSpeedControl.setSpeed(0.75); }); it('set the new speed as active', function () { - expect($('.video-speeds li[data-speed="1.0"]')) - .not.toHaveClass('is-active'); - expect($('.video-speeds li[data-speed="0.75"]')) - .toHaveClass('is-active'); - expect($('.speeds .value')).toHaveHtml('0.75x'); + expect($('li[data-speed="1.0"]')).not.toHaveClass('is-active'); + expect($('li[data-speed="1.0"] .speed-option').attr('aria-pressed')).not.toEqual('true'); + + expect($('li[data-speed="0.75"]')).toHaveClass('is-active'); + expect($('li[data-speed="0.75"] .speed-option').attr('aria-pressed')).toEqual('true'); + + expect($('.speeds .speed-button .value')).toHaveHtml('0.75x'); }); }); diff --git a/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js b/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js index 986d9ea7ff..ace6f39114 100644 --- a/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js @@ -1,9 +1,10 @@ (function (requirejs, require, define) { "use strict"; define( -'video/08_video_speed_control.js', -['video/00_iterator.js'], -function (Iterator) { +'video/08_video_speed_control.js', [ + 'video/00_iterator.js', + 'edx-ui-toolkit/js/utils/html-utils' +], function (Iterator, HtmlUtils) { /** * Video speed control module. * @exports video/08_video_speed_control.js @@ -95,23 +96,38 @@ function (Iterator) { * Creates any necessary DOM elements, attach them, and set their, * initial configuration. * @param {array} speeds List of speeds available for the player. + * @param {string} currentSpeed The current speed set to the player. */ - render: function (speeds) { + render: function (speeds, currentSpeed) { var speedsContainer = this.speedsContainer, reversedSpeeds = speeds.concat().reverse(), speedsList = $.map(reversedSpeeds, function (speed) { - return [ - '
  • ', - '', - '
  • ' - ].join(''); + return HtmlUtils.interpolateHtml( + HtmlUtils.HTML( + [ + '
  • ', + '', + '
  • ' + ].join('') + ), + { + speed: speed + } + ).toString(); }); - speedsContainer.html(speedsList.join('')); + HtmlUtils.setHtml( + speedsContainer, + HtmlUtils.HTML(speedsList) + ); this.speedLinks = new Iterator(speedsContainer.find('.speed-option')); - this.state.el.find('.secondary-controls').prepend(this.el); + HtmlUtils.prepend( + this.state.el.find('.secondary-controls'), + HtmlUtils.HTML(this.el) + ); + this.setActiveSpeed(currentSpeed); }, /** @@ -216,17 +232,38 @@ function (Iterator) { if (speed !== this.currentSpeed || forceUpdate) { this.speedsContainer .find('li') - .removeClass('is-active') - .siblings("li[data-speed='" + speed + "']") - .addClass('is-active'); + .siblings("li[data-speed='" + speed + "']"); - this.speedButton.find('.value').html(speed + 'x'); + this.speedButton.find('.value').text(speed + 'x'); this.currentSpeed = speed; if (!silent) { this.el.trigger('speedchange', [speed, this.state.speed]); } } + + this.resetActiveSpeed(); + this.setActiveSpeed(speed); + }, + + resetActiveSpeed: function() { + var speedOptions = this.speedsContainer.find('li'); + + $(speedOptions).each(function(index, el) { + $(el).removeClass('is-active') + .find('.speed-option') + .attr('aria-pressed', 'false'); + }); + }, + + setActiveSpeed: function(speed) { + var speedOption = this.speedsContainer.find('li[data-speed="' + speed + '"]'); + + speedOption.addClass('is-active') + .find('.speed-option') + .attr('aria-pressed', 'true'); + + this.speedButton.attr('title', gettext('Video speed: ') + speed + 'x'); }, /** @@ -244,10 +281,13 @@ function (Iterator) { * @param {jquery Event} event */ clickLinkHandler: function (event) { - var speed = $(event.currentTarget).parent().data('speed'); - - this.closeMenu(); + var el = $(event.currentTarget).parent(), + speed = $(el).data('speed'); + + this.resetActiveSpeed(); + this.setActiveSpeed(speed); this.state.videoCommands.execute('speed', speed); + this.closeMenu(true); return false; }, diff --git a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js index b99a1b1128..ec7e677372 100644 --- a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js +++ b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js @@ -5,11 +5,12 @@ define('video/09_video_caption.js',[ 'video/00_sjson.js', 'video/00_async_process.js', + 'edx-ui-toolkit/js/utils/html-utils', 'draggabilly', 'modernizr', 'afontgarde', 'edxicons' - ], function (Sjson, AsyncProcess, Draggabilly) { + ], function (Sjson, AsyncProcess, HtmlUtils, Draggabilly) { /** * @desc VideoCaption module exports a function. @@ -80,47 +81,60 @@ renderElements: function () { var languages = this.state.config.transcriptLanguages; - var langTemplate = [ - '
    ', - '', - '', - '', - '
    ' - ].join(''); + var langHtml = HtmlUtils.interpolateHtml( + HtmlUtils.HTML( + [ + '
    ', + '', + '', + '', + '
    ' + ].join(''), + { + langTitle: gettext('Open language menu') + } + ) + + ); - var template = [ - '
    ', - '

    ', - '
      ', - '
      ' - ].join(''); + var subtitlesHtml = HtmlUtils.interpolateHtml( + HtmlUtils.HTML( + [ + '
      ', + '

      ', + '
        ', + '
        ' + ].join('')), + { + courseId: this.state.id, + courseLang: this.state.lang + } + ); this.loaded = false; - this.subtitlesEl = $(template); + this.subtitlesEl = $(HtmlUtils.ensureHtml(subtitlesHtml).toString()); this.subtitlesMenuEl = this.subtitlesEl.find('.subtitles-menu'); - this.container = $(langTemplate); + this.container = $(HtmlUtils.ensureHtml(langHtml).toString()); this.captionControlEl = this.container.find('.toggle-captions'); this.captionDisplayEl = this.state.el.find('.closed-captions'); this.transcriptControlEl = this.container.find('.toggle-transcript'); @@ -542,15 +556,26 @@ } } else { if (state.isTouch) { - self.subtitlesEl.find('.subtitles-menu') - .text(gettext('Transcript will be displayed when you start playing the video.')) // jshint ignore: line - .wrapInner('
      1. '); + HtmlUtils.setHtml( + self.subtitlesEl.find('.subtitles-menu'), + HtmlUtils.joinHtml( + HtmlUtils.HTML('
      2. '), + gettext('Transcript will be displayed when you start playing the video.'), + HtmlUtils.HTML('
      3. ') + ) + ); } else { self.renderCaption(start, captions); } self.hideCaptions(state.hide_captions, false); - self.state.el.find('.video-wrapper').after(self.subtitlesEl); - self.state.el.find('.secondary-controls').append(self.container); + HtmlUtils.append( + self.state.el.find('.video-wrapper').parent(), + HtmlUtils.HTML(self.subtitlesEl) + ); + HtmlUtils.append( + self.state.el.find('.secondary-controls'), + HtmlUtils.HTML(self.container) + ); self.bindHandlers(); } @@ -630,9 +655,11 @@ onResize: function () { this.subtitlesEl .find('.spacing').first() - .height(this.topSpacingHeight()).end() + .height(this.topSpacingHeight()); + + this.subtitlesEl .find('.spacing').last() - .height(this.bottomSpacingHeight()); + .height(this.bottomSpacingHeight()); this.scrollCaption(); this.setSubtitlesHeight(); @@ -649,8 +676,9 @@ renderLanguageMenu: function (languages) { var self = this, state = this.state, - menu = $('