Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix Studio to gracefully handle xblock JavaScript errors

Browse Source 
TNL-46

I've changed Studio to catch JavaScript errors when rendering xblocks, log the error, but to then continue as normal. This means that the user is still able to interact with the xblock to delete, duplicate etc. This seems reasonable as the xblock is only rendered as a WYSIWYG representation so if it isn't fully interactive that shouldn't be a big problem.
This commit is contained in:
Andy Armstrong
2014-08-20 12:18:02 -04:00
parent fa2154a694
commit d451c3e91d
13 changed files with 281 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cms/static/js/spec/views/modals/edit_xblock_spec.js
View File

@@ -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();
});

59
cms/static/js/spec/views/pages/container_spec.js
View File

@@ -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);

15
cms/static/js/views/modals/edit_xblock.js
View File

@@ -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) {

39
cms/static/js/views/pages/container.js
View File

@@ -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 $("<div/>", { 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 = $('<div class="studio-xblock-wrapper"></div>').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&hellip;'),
function() {
var scrollOffset = ViewUtils.getScrollOffset(xblockElement),
placeholderElement = $('<div class="studio-xblock-wrapper"></div>').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();
},

65
cms/static/js/views/xblock.js
View File

@@ -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;