diff --git a/.jshintrc b/.jshintrc index be4e6a949d..2e88ababdd 100644 --- a/.jshintrc +++ b/.jshintrc @@ -22,12 +22,12 @@ "nonbsp" : true, // Warns about "non-breaking whitespace" characters. "nonew" : true, // Prohibits the use of constructor functions for side-effects. "plusplus" : false, // Prohibits the use of unary increment and decrement operators. - "quotmark" : "single", // Enforces the consistency of quotation marks used throughout your code. It accepts three values: true, "single", and "double". + "quotmark" : false, // Enforces the consistency of quotation marks used throughout your code. It accepts three values: true, "single", and "double". "undef" : true, // Prohibits the use of explicitly undeclared variables. "unused" : true, // Warns when you define and never use your variables. "strict" : true, // Requires all functions to run in ECMAScript 5's strict mode. "trailing" : true, // Makes it an error to leave a trailing whitespace in your code. - "maxlen" : 80, // Lets you set the maximum length of a line. + "maxlen" : 120, // Lets you set the maximum length of a line. //"maxparams" : 4, // Lets you set the max number of formal parameters allowed per function. //"maxdepth" : 4, // Lets you control how nested do you want your blocks to be. //"maxstatements" : 4, // Lets you set the max number of statements allowed per function. @@ -59,7 +59,7 @@ "shadow" : false, // Suppresses warnings about variable shadowing i.e. declaring a variable that had been already declared somewhere in the outer scope. "sub" : false, // Suppresses warnings about using [] notation when it can be expressed in dot notation. "supernew" : false, // Suppresses warnings about "weird" constructions like new function () { ... } and new Object;. - "validthis" : true, // Suppresses warnings about possible strict violations when the code is running in strict mode and you use this in a non-constructor function. + "validthis" : true, // Suppresses warnings about possible strict violations when the code is running in strict mode and you use this in a non-constructor function. "noyield" : false, // Suppresses warnings about generator functions with no yield statement in them. @@ -73,11 +73,11 @@ // The rest should remain `false`. Please see explanation for the "predef" parameter below. "couch" : false, // Defines globals exposed by CouchDB. "dojo" : false, // Defines globals exposed by the Dojo Toolkit - "jquery" : false, // Defines globals exposed by the jQuery JavaScript library. + "jquery" : false, // Defines globals exposed by the jQuery JavaScript library. "mootools" : false, // Defines globals exposed by the MooTools JavaScript framework. "node" : false, // Defines globals available when your code is running inside of the Node runtime environment. "nonstandard" : false, // Defines non-standard but widely adopted globals such as escape and unescape. - "phantom" : false, // Defines globals available when your core is running inside of the PhantomJS runtime environment. + "phantom" : false, // Defines globals available when your core is running inside of the PhantomJS runtime environment. "prototypejs" : false, // Defines globals exposed by the Prototype JavaScript framework. "rhino" : false, // Defines globals available when your code is running inside of the Rhino runtime environment. "worker" : false, // Defines globals available when your code is running inside of a Web Worker. diff --git a/cms/static/js/spec/views/modals/edit_xblock_spec.js b/cms/static/js/spec/views/modals/edit_xblock_spec.js index ecc12b43f6..301c1b8a44 100644 --- a/cms/static/js/spec/views/modals/edit_xblock_spec.js +++ b/cms/static/js/spec/views/modals/edit_xblock_spec.js @@ -71,8 +71,8 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers refreshed = true; }; modal = showModal(requests, mockXBlockEditorHtml, { refresh: refresh }); - modal.runtime.notify('save', { state: 'start' }); - modal.runtime.notify('save', { state: 'end' }); + modal.editorView.notifyRuntime('save', { state: 'start' }); + modal.editorView.notifyRuntime('save', { state: 'end' }); expect(edit_helpers.isShowingModal(modal)).toBeFalsy(); expect(refreshed).toBeTruthy(); }); @@ -84,7 +84,7 @@ define(["jquery", "underscore", "js/spec_helpers/create_sinon", "js/spec_helpers refreshed = true; }; modal = showModal(requests, mockXBlockEditorHtml, { refresh: refresh }); - modal.runtime.notify('cancel'); + modal.editorView.notifyRuntime('cancel'); expect(edit_helpers.isShowingModal(modal)).toBeFalsy(); expect(refreshed).toBeFalsy(); }); diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index cd93a86fc6..1089ec47a6 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -7,6 +7,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin model, containerPage, requests, initialDisplayName, mockContainerPage = readFixtures('mock/mock-container-page.underscore'), mockContainerXBlockHtml = readFixtures('mock/mock-container-xblock.underscore'), + mockBadContainerXBlockHtml = readFixtures('mock/mock-bad-javascript-container-xblock.underscore'), + mockBadXBlockContainerXBlockHtml = readFixtures('mock/mock-bad-xblock-container-xblock.underscore'), mockUpdatedContainerXBlockHtml = readFixtures('mock/mock-updated-container-xblock.underscore'), mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'); @@ -15,6 +17,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin edit_helpers.installEditTemplates(); edit_helpers.installTemplate('xblock-string-field-editor'); + edit_helpers.installTemplate('container-message'); appendSetFixtures(mockContainerPage); edit_helpers.installMockXBlock({ @@ -83,6 +86,18 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden'); }); + it('can show an xblock with broken JavaScript', function() { + renderContainerPage(this, mockBadContainerXBlockHtml); + expect(containerPage.$('.wrapper-xblock .level-nesting')).not.toHaveClass('is-hidden'); + expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden'); + }); + + it('can show an xblock with an invalid XBlock', function() { + renderContainerPage(this, mockBadXBlockContainerXBlockHtml); + expect(containerPage.$('.wrapper-xblock .level-nesting')).not.toHaveClass('is-hidden'); + expect(containerPage.$('.ui-loading')).toHaveClass('is-hidden'); + }); + it('inline edits the display name when performing a new action', function() { renderContainerPage(this, mockContainerXBlockHtml, { action: 'new' @@ -138,6 +153,9 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin resources: [] }); + // Respond to the subsequent xblock info fetch request. + create_sinon.respondWithJson(requests, {"display_name": updatedDisplayName}); + // Expect the title to have been updated expect(displayNameElement.text().trim()).toBe(updatedDisplayName); }); @@ -177,6 +195,18 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin }); expect(edit_helpers.isShowingModal()).toBeTruthy(); }); + + it('can show an edit modal for a child xblock with broken JavaScript', function() { + var editButtons; + renderContainerPage(this, mockBadContainerXBlockHtml); + editButtons = containerPage.$('.wrapper-xblock .edit-button'); + editButtons[0].click(); + create_sinon.respondWithJson(requests, { + html: mockXBlockEditorHtml, + resources: [] + }); + expect(edit_helpers.isShowingModal()).toBeTruthy(); + }); }); describe("Editing an xmodule", function() { @@ -268,10 +298,10 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin clickDelete(componentIndex); create_sinon.respondWithJson(requests, {}); - // first request contains given component's id (to delete the component) - expect(requests[requests.length - 2].url).toMatch( - new RegExp("locator-component-" + GROUP_TO_TEST + (componentIndex + 1)) - ); + // second to last request contains given component's id (to delete the component) + create_sinon.expectJsonRequest(requests, 'DELETE', + '/xblock/locator-component-' + GROUP_TO_TEST + (componentIndex + 1), + null, requests.length - 2); // final request to refresh the xblock info create_sinon.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); @@ -302,6 +332,18 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin deleteComponentWithSuccess(NUM_COMPONENTS_PER_GROUP - 1); }); + it("can delete an xblock with broken JavaScript", function() { + renderContainerPage(this, mockBadContainerXBlockHtml); + containerPage.$('.delete-button').first().click(); + edit_helpers.confirmPrompt(promptSpy); + create_sinon.respondWithJson(requests, {}); + // expect the second to last request to be a delete of the xblock + create_sinon.expectJsonRequest(requests, 'DELETE', '/xblock/locator-broken-javascript', + null, requests.length - 2); + // expect the last request to be a fetch of the xblock info for the parent container + create_sinon.expectJsonRequest(requests, 'GET', '/xblock/locator-container'); + }); + it('does not delete when clicking No in prompt', function () { var numRequests; @@ -387,6 +429,15 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin duplicateComponentWithSuccess(NUM_COMPONENTS_PER_GROUP - 1); }); + it("can duplicate an xblock with broken JavaScript", function() { + renderContainerPage(this, mockBadContainerXBlockHtml); + containerPage.$('.duplicate-button').first().click(); + create_sinon.expectJsonRequest(requests, 'POST', '/xblock/', { + 'duplicate_source_locator': 'locator-broken-javascript', + 'parent_locator': 'locator-container' + }); + }); + it('shows a notification when duplicating', function () { var notificationSpy = edit_helpers.createNotificationSpy(); renderContainerPage(this, mockContainerXBlockHtml); diff --git a/cms/static/js/views/modals/edit_xblock.js b/cms/static/js/views/modals/edit_xblock.js index 99698535d3..67e9de6f88 100644 --- a/cms/static/js/views/modals/edit_xblock.js +++ b/cms/static/js/views/modals/edit_xblock.js @@ -65,15 +65,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie onDisplayXBlock: function() { var editorView = this.editorView, - title = this.getTitle(), - xblock = editorView.xblock, - runtime = xblock.runtime; + title = this.getTitle(); // Notify the runtime that the modal has been shown - if (runtime) { - this.runtime = runtime; - runtime.notify('modal-shown', this); - } + editorView.notifyRuntime('modal-shown', this); // Update the modal's header if (editorView.hasCustomTabs()) { @@ -93,7 +88,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie // If the xblock is not using custom buttons then choose which buttons to show if (!editorView.hasCustomButtons()) { // If the xblock does not support save then disable the save button - if (!xblock.save) { + if (!editorView.xblock.save) { this.disableSave(); } this.getActionBar().show(); @@ -175,9 +170,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie BaseModal.prototype.hide.call(this); // Notify the runtime that the modal has been hidden - if (this.runtime) { - this.runtime.notify('modal-hidden'); - } + this.editorView.notifyRuntime('modal-hidden'); }, findXBlockInfo: function(xblockWrapperElement, defaultXBlockInfo) { diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 257b76fe15..bce220ee2b 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -9,6 +9,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views function ($, _, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, EditXBlockModal, XBlockInfo, XBlockStringFieldEditor, ContainerSubviews, UnitOutlineView, XBlockUtils) { + 'use strict'; var XBlockContainerPage = BasePage.extend({ // takes XBlockInfo as a model @@ -88,14 +89,22 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views // Render the xblock xblockView.render({ - success: function() { - xblockView.xblock.runtime.notify("page-shown", self); + done: function() { + // Show the xblock and hide the loading indicator xblockView.$el.removeClass(hiddenCss); - self.renderAddXBlockComponents(); - self.onXBlockRefresh(xblockView); - self.refreshDisplayName(); loadingElement.addClass(hiddenCss); + + // Notify the runtime that the page has been successfully shown + xblockView.notifyRuntime('page-shown', self); + + // Render the add buttons + self.renderAddXBlockComponents(); + + // Refresh the views now that the xblock is visible + self.onXBlockRefresh(xblockView); unitLocationTree.removeClass(hiddenCss); + + // Re-enable Backbone events for any updated DOM elements self.delegateEvents(); } }); @@ -109,11 +118,6 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views return this.xblockView.model.urlRoot; }, - refreshDisplayName: function() { - var displayName = this.$('.xblock-header .header-details .xblock-display-name').first().text().trim(); - this.model.set('display_name', displayName); - }, - onXBlockRefresh: function(xblockView) { this.addButtonActions(xblockView.$el); this.xblockView.refresh(); @@ -159,6 +163,10 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views }); }, + createPlaceholderElement: function() { + return $("
", { class: "studio-xblock-wrapper" }); + }, + createComponent: function(template, target) { // A placeholder element is created in the correct location for the new xblock // and then onNewXBlock will replace it with a rendering of the xblock. Note that @@ -168,7 +176,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views buttonPanel = target.closest('.add-xblock-component'), listPanel = buttonPanel.prev(), scrollOffset = ViewUtils.getScrollOffset(buttonPanel), - placeholderElement = $('
').appendTo(listPanel), + placeholderElement = this.createPlaceholderElement().appendTo(listPanel), requestData = _.extend(template, { parent_locator: parentLocator }); @@ -189,7 +197,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views ViewUtils.runOperationShowingMessage(gettext('Duplicating…'), function() { var scrollOffset = ViewUtils.getScrollOffset(xblockElement), - placeholderElement = $('
').insertAfter(xblockElement), + placeholderElement = self.createPlaceholderElement().insertAfter(xblockElement), parentElement = self.findXBlockElement(parent), requestData = { duplicate_source_locator: xblockElement.data('locator'), @@ -217,10 +225,13 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views onDelete: function(xblockElement) { // get the parent so we can remove this component from its parent. var xblockView = this.xblockView, - xblock = xblockView.xblock, parent = this.findXBlockElement(xblockElement.parent()); xblockElement.remove(); - xblock.runtime.notify('deleted-child', parent.data('locator')); + + // Inform the runtime that the child has been deleted in case + // other views are listening to deletion events. + xblockView.notifyRuntime('deleted-child', parent.data('locator')); + // Update publish and last modified information from the server. this.model.fetch(); }, diff --git a/cms/static/js/views/xblock.js b/cms/static/js/views/xblock.js index 48906f0b58..78d1f08b84 100644 --- a/cms/static/js/views/xblock.js +++ b/cms/static/js/views/xblock.js @@ -29,34 +29,58 @@ define(["jquery", "underscore", "js/views/baseview", "xblock/runtime.v1"], var self = this, wrapper = this.$el, xblockElement, - success = options ? options.success : null, + successCallback = options ? options.success || options.done : null, + errorCallback = options ? options.error || options.done : null, xblock, fragmentsRendered; fragmentsRendered = this.renderXBlockFragment(fragment, wrapper); - fragmentsRendered.done(function() { + fragmentsRendered.always(function() { xblockElement = self.$('.xblock').first(); - xblock = XBlock.initializeBlock(xblockElement); - self.xblock = xblock; - self.xblockReady(xblock); - if (success) { - success(xblock); + try { + xblock = XBlock.initializeBlock(xblockElement); + self.xblock = xblock; + self.xblockReady(xblock); + if (successCallback) { + successCallback(xblock); + } + } catch (e) { + console.error(e.stack); + // Add 'xblock-initialization-failed' class to every xblock + self.$('.xblock').addClass('xblock-initialization-failed'); + + // If the xblock was rendered but failed then still call xblockReady to allow + // drag-and-drop to be initialized. + if (xblockElement) { + self.xblockReady(null); + } + if (errorCallback) { + errorCallback(); + } } }); }, /** - * This method is called upon successful rendering of an xblock. + * Sends a notification event to the runtime, if one is available. Note that the runtime + * is only available once the xblock has been rendered and successfully initialized. + * @param eventName The name of the event to be fired. + * @param data The data to be passed to any listener's of the event. */ - xblockReady: function(xblock) { - // Do nothing + notifyRuntime: function(eventName, data) { + var runtime = this.xblock && this.xblock.runtime; + if (runtime) { + runtime.notify(eventName, data); + } }, /** - * Returns true if the specified xblock has children. + * This method is called upon successful rendering of an xblock. Note that the xblock + * may have thrown JavaScript errors after rendering in which case the xblock parameter + * will be null. */ - hasChildXBlocks: function() { - return this.$('.wrapper-xblock').length > 0; + xblockReady: function(xblock) { + // Do nothing }, /** @@ -77,9 +101,16 @@ define(["jquery", "underscore", "js/views/baseview", "xblock/runtime.v1"], } // Render the HTML first as the scripts might depend upon it, and then - // asynchronously add the resources to the page. - this.updateHtml(element, html); - return this.addXBlockFragmentResources(resources); + // asynchronously add the resources to the page. Any errors that are thrown + // by included scripts are logged to the console but are then ignored assuming + // that at least the rendered HTML will be in place. + try { + this.updateHtml(element, html); + return this.addXBlockFragmentResources(resources); + } catch(e) { + console.error(e.stack); + return $.Deferred().resolve(); + } }, /** @@ -106,7 +137,7 @@ define(["jquery", "underscore", "js/views/baseview", "xblock/runtime.v1"], numResources = resources.length; deferred = $.Deferred(); applyResource = function(index) { - var hash, resource, head, value, promise; + var hash, resource, value, promise; if (index >= numResources) { deferred.resolve(); return; diff --git a/cms/templates/js/mock/mock-bad-javascript-container-xblock.underscore b/cms/templates/js/mock/mock-bad-javascript-container-xblock.underscore new file mode 100644 index 0000000000..032655ab31 --- /dev/null +++ b/cms/templates/js/mock/mock-bad-javascript-container-xblock.underscore @@ -0,0 +1,54 @@ +
+
+
+ Test Container +
+
+
    +
+
+
+
+
+
+
    +
  1. +
    +
    +
    +
    +
      +
    • + +
      • +
    • + +
      • +
    • + +
      • +
    • + +
      • +
    +
    +
    +
    + +
    +
    + +
    +
    +
    +
    2. +
+
+
diff --git a/cms/templates/js/mock/mock-bad-xblock-container-xblock.underscore b/cms/templates/js/mock/mock-bad-xblock-container-xblock.underscore new file mode 100644 index 0000000000..77b0ee062d --- /dev/null +++ b/cms/templates/js/mock/mock-bad-xblock-container-xblock.underscore @@ -0,0 +1,51 @@ +
+
+
+ Test Container +
+
+
    +
+
+
+
+
+
+
    +
  1. +
    +
    +
    +
    +
      +
    • + +
      • +
    • + +
      • +
    • + +
      • +
    • + +
      • +
    +
    +
    +
    + +
    +
    +
    +
    +
    +
    2. +
+
+
diff --git a/cms/templates/js/mock/mock-container-xblock.underscore b/cms/templates/js/mock/mock-container-xblock.underscore index ac2f3cbb65..468e82c6ac 100644 --- a/cms/templates/js/mock/mock-container-xblock.underscore +++ b/cms/templates/js/mock/mock-container-xblock.underscore @@ -14,7 +14,7 @@ data-init="MockXBlock" data-runtime-class="StudioRuntime" data-runtime-version="1">
  1. -
    +
    @@ -38,8 +38,7 @@
    1. -
      +
      @@ -64,9 +63,7 @@
    2. -
      - +
      @@ -91,8 +88,7 @@
    3. -
      +
      @@ -123,7 +119,7 @@
    4. -
      +
      @@ -147,9 +143,7 @@
      1. -
        - +
        @@ -174,9 +168,7 @@
      2. -
        - +
        @@ -201,9 +193,7 @@
      3. -
        - +
        diff --git a/cms/templates/js/mock/mock-xmodule-editor-with-custom-tabs.underscore b/cms/templates/js/mock/mock-xmodule-editor-with-custom-tabs.underscore index a9f5fe1b34..b37459e21f 100644 --- a/cms/templates/js/mock/mock-xmodule-editor-with-custom-tabs.underscore +++ b/cms/templates/js/mock/mock-xmodule-editor-with-custom-tabs.underscore @@ -1,11 +1,17 @@ -
        +
        diff --git a/cms/templates/js/mock/mock-xmodule-editor.underscore b/cms/templates/js/mock/mock-xmodule-editor.underscore index 8c179ef704..38bc6f197e 100644 --- a/cms/templates/js/mock/mock-xmodule-editor.underscore +++ b/cms/templates/js/mock/mock-xmodule-editor.underscore @@ -1,4 +1,6 @@ -
        +
        @@ -24,7 +26,8 @@ -
        +
        diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 9d89cc3d13..b4eb6a498e 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -37,6 +37,10 @@ class ContainerPage(PageObject): return None def is_browser_on_page(self): + def _xblock_count(class_name, request_token): + return len(self.q(css='{body_selector} .xblock.{class_name}[data-request-token="{request_token}"]'.format( + body_selector=XBlockWrapper.BODY_SELECTOR, class_name=class_name, request_token=request_token + )).results) def _is_finished_loading(): is_done = False @@ -46,11 +50,15 @@ class ContainerPage(PageObject): request_token = data_request_elements.first.attrs('data-request-token')[0] # Then find the number of Studio xblock wrappers on the page with that request token. num_wrappers = len(self.q(css='{} [data-request-token="{}"]'.format(XBlockWrapper.BODY_SELECTOR, request_token)).results) - # Wait until all components have been loaded. - # See common/static/coffee/src/xblock/core.coffee which adds the - # class "xblock-initialized" at the end of initializeBlock - num_xblocks_init = len(self.q(css='{} .xblock.xblock-initialized[data-request-token="{}"]'.format(XBlockWrapper.BODY_SELECTOR, request_token)).results) - is_done = num_wrappers == num_xblocks_init + # Wait until all components have been loaded and marked as either initialized or failed. + # See: + # - common/static/coffee/src/xblock/core.coffee which adds the class "xblock-initialized" + # at the end of initializeBlock. + # - common/static/js/views/xblock.js which adds the class "xblock-initialization-failed" + # if the xblock threw an error while initializing. + num_initialized_xblocks = _xblock_count('xblock-initialized', request_token) + num_failed_xblocks = _xblock_count('xblock-initialization-failed', request_token) + is_done = num_wrappers == (num_initialized_xblocks + num_failed_xblocks) return (is_done, is_done) # First make sure that an element with the view-container class is present on the page, diff --git a/common/test/acceptance/tests/test_studio_bad_data.py b/common/test/acceptance/tests/test_studio_bad_data.py index 609e3719cb..a42a45ebd0 100644 --- a/common/test/acceptance/tests/test_studio_bad_data.py +++ b/common/test/acceptance/tests/test_studio_bad_data.py @@ -95,8 +95,7 @@ class JSErrorBadContentTest(BadComponentTest): """ Tests that components that throw JS errors do not break the Unit page. """ - # TODO: ENABLE TEST WITH ANDY'S PR - __test__ = False + __test__ = True def get_bad_html_content(self): """