Merge pull request #3046 from edx/andya/fix-xblock-loading-in-studio

Fix Studio's XBlock dependency loading issues

cms/static/coffee/spec/views/module_edit_spec.coffee

@@ -58,7 +58,7 @@ define ["jquery", "coffee/src/views/module_edit", "js/models/module_info", "xmod
             spyOn(@moduleEdit, 'loadEdit')
             spyOn(@moduleEdit, 'delegateEvents')
             spyOn($.fn, 'append')
-            spyOn($, 'getScript')
+            spyOn($, 'getScript').andReturn($.Deferred().resolve().promise())
 
             window.loadedXBlockResources = undefined
 

cms/static/coffee/src/views/module_edit.coffee

@@ -81,8 +81,7 @@ define ["jquery", "underscore", "gettext", "xblock/runtime.v1",
           headers:
             Accept: 'application/json'
           success: (fragment) =>
-            @renderXBlockFragment(fragment, target, viewName)
-            callback()
+            @renderXBlockFragment(fragment, target).done(callback)
         )
 
     render: -> @loadView('student_view', @$el, =>

cms/static/js/spec/views/xblock_spec.js

@@ -38,18 +38,22 @@ define([ "jquery", "js/spec/create_sinon", "URI", "js/views/xblock", "js/models/
                 var postXBlockRequest;
 
                 postXBlockRequest = function(requests, resources) {
+                    var promise;
                     $.ajax({
                         url: "test_url",
                         type: 'GET',
                         success: function(fragment) {
-                            xblockView.renderXBlockFragment(fragment, this.$el);
+                            promise = xblockView.renderXBlockFragment(fragment, this.$el);
                         }
                     });
+                    // Note: this mock response will call the AJAX success function synchronously
+                    // so the promise variable defined above will be available.
                     respondWithMockXBlockFragment(requests, {
                         html: mockXBlockHtml,
                         resources: resources
                     });
                     expect(xblockView.$el.select('.xblock-header')).toBeTruthy();
+                    return promise;
                 };
 
                 it('can render an xblock with no CSS or JavaScript', function() {
@@ -87,6 +91,17 @@ define([ "jquery", "js/spec/create_sinon", "URI", "js/views/xblock", "js/models/
                     ]);
                     expect($('head').html()).toContain(mockHeadTag);
                 });
+
+                it('aborts rendering when a dependent script fails to load', function() {
+                    var requests = create_sinon.requests(this),
+                        mockJavaScriptUrl = "mock.js",
+                        promise;
+                    spyOn($, 'getScript').andReturn($.Deferred().reject().promise());
+                    promise = postXBlockRequest(requests, [
+                        ["hash5", { mimetype: "application/javascript", kind: "url", data: mockJavaScriptUrl }]
+                    ]);
+                    expect(promise.isRejected()).toBe(true);
+                });
             });
         });
     });

cms/static/js/views/xblock.js

@@ -21,64 +21,110 @@ define(["jquery", "underscore", "js/views/baseview", "xblock/runtime.v1"],
                     success: function(fragment) {
                         var wrapper = self.$el,
                             xblock;
-                        self.renderXBlockFragment(fragment, wrapper);
-                        xblock = self.$('.xblock').first();
-                        XBlock.initializeBlock(xblock);
+                        self.renderXBlockFragment(fragment, wrapper).done(function() {
+                            xblock = self.$('.xblock').first();
+                            XBlock.initializeBlock(xblock);
+                            self.delegateEvents();
+                        });
                     }
                 });
             },
 
 
             /**
-             * Renders an xblock fragment into the specifed element. The fragment has two attributes:
+             * Renders an xblock fragment into the specified element. The fragment has two attributes:
              *   html: the HTML to be rendered
              *   resources: any JavaScript or CSS resources that the HTML depends upon
+             * Note that the XBlock is rendered asynchronously, and so a promise is returned that
+             * represents this process.
              * @param fragment The fragment returned from the xblock_handler
              * @param element The element into which to render the fragment (defaults to this.$el)
+             * @returns {*} A promise representing the rendering process
              */
             renderXBlockFragment: function(fragment, element) {
-                var applyResource, i, len, resources, resource;
+                var html = fragment.html,
+                    resources = fragment.resources;
                 if (!element) {
                     element = this.$el;
                 }
+                // First render the HTML as the scripts might depend upon it
+                element.html(html);
+                // Now asynchronously add the resources to the page
+                return this.addXBlockFragmentResources(resources);
+            },
 
-                applyResource = function(value) {
-                    var hash, resource, head;
+            /**
+             * Dynamically loads all of an XBlock's dependent resources. This is an asynchronous
+             * process so a promise is returned.
+             * @param resources The resources to be rendered
+             * @returns {*} A promise representing the rendering process
+             */
+            addXBlockFragmentResources: function(resources) {
+                var self = this,
+                    applyResource,
+                    numResources,
+                    deferred;
+                numResources = resources.length;
+                deferred = $.Deferred();
+                applyResource = function(index) {
+                    var hash, resource, head, value, promise;
+                    if (index >= numResources) {
+                        deferred.resolve();
+                        return;
+                    }
+                    value = resources[index];
                     hash = value[0];
                     if (!window.loadedXBlockResources) {
                         window.loadedXBlockResources = [];
                     }
                     if (_.indexOf(window.loadedXBlockResources, hash) < 0) {
                         resource = value[1];
-                        head = $('head');
-                        if (resource.mimetype === "text/css") {
-                            if (resource.kind === "text") {
-                                head.append("<style type='text/css'>" + resource.data + "</style>");
-                            } else if (resource.kind === "url") {
-                                head.append("<link rel='stylesheet' href='" + resource.data + "' type='text/css'>");
-                            }
-                        } else if (resource.mimetype === "application/javascript") {
-                            if (resource.kind === "text") {
-                                head.append("<script>" + resource.data + "</script>");
-                            } else if (resource.kind === "url") {
-                                $.getScript(resource.data);
-                            }
-                        } else if (resource.mimetype === "text/html") {
-                            if (resource.placement === "head") {
-                                head.append(resource.data);
-                            }
-                        }
+                        promise = self.loadResource(resource);
                         window.loadedXBlockResources.push(hash);
+                        promise.done(function() {
+                            applyResource(index + 1);
+                        }).fail(function() {
+                            deferred.reject();
+                        });
+                    } else {
+                        applyResource(index + 1);
                     }
                 };
+                applyResource(0);
+                return deferred.promise();
+            },
 
-                element.html(fragment.html);
-                resources = fragment.resources;
-                for (i = 0, len = resources.length; i < len; i++) {
-                    resource = resources[i];
-                    applyResource(resource);
+            /**
+             * Loads the specified resource into the page.
+             * @param resource The resource to be loaded.
+             * @returns {*} A promise representing the loading of the resource.
+             */
+            loadResource: function(resource) {
+                var head = $('head'),
+                    mimetype = resource.mimetype,
+                    kind = resource.kind,
+                    placement = resource.placement,
+                    data = resource.data;
+                if (mimetype === "text/css") {
+                    if (kind === "text") {
+                        head.append("<style type='text/css'>" + data + "</style>");
+                    } else if (kind === "url") {
+                        head.append("<link rel='stylesheet' href='" + data + "' type='text/css'>");
+                    }
+                } else if (mimetype === "application/javascript") {
+                    if (kind === "text") {
+                        head.append("<script>" + data + "</script>");
+                    } else if (kind === "url") {
+                        // Return a promise for the script resolution
+                        return $.getScript(data);
+                    }
+                } else if (mimetype === "text/html") {
+                    if (placement === "head") {
+                        head.append(data);
+                    }
                 }
-                return this.delegateEvents();
+                // Return an already resolved promise for synchronous updates
+                return $.Deferred().resolve().promise();
             }
         });