diff --git a/notebook/static/notebook/js/cell.js b/notebook/static/notebook/js/cell.js index 2c98082a6..d94501389 100644 --- a/notebook/static/notebook/js/cell.js +++ b/notebook/static/notebook/js/cell.js @@ -350,16 +350,6 @@ define([ } }; - /** - * Garbage collects unused attachments in this cell - * @method remove_unused_attachments - */ - Cell.prototype.remove_unused_attachments = function () { - // Cell subclasses which support attachments should override this - // and keep them when needed - this.attachments = {}; - }; - /** * Delegates keyboard shortcut handling to either Jupyter keyboard * manager when in command mode, or CodeMirror when in edit mode @@ -756,11 +746,6 @@ define([ this.element = cell; }; - UnrecognizedCell.prototype.remove_unused_attachments = function () { - // Do nothing to avoid removing attachments from a possible future - // attachment-supporting cell type - }; - UnrecognizedCell.prototype.bind_events = function () { Cell.prototype.bind_events.apply(this, arguments); var cell = this; diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index 30ea63362..250db8bf8 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -2403,19 +2403,9 @@ define(function (require) { }; /** - * Garbage collects unused attachments in all the cells - */ - Notebook.prototype.remove_unused_attachments = function() { - var cells = this.get_cells(); - for (var i = 0; i < cells.length; i++) { - var cell = cells[i]; - cell.remove_unused_attachments(); - } - }; - - /** + Move the unused attachments garbage collection logic to TextCell.toJSON. * Load a notebook from JSON (.ipynb). - * + * * @param {object} data - JSON representation of a notebook */ Notebook.prototype.fromJSON = function (data) { @@ -2478,7 +2468,7 @@ define(function (require) { if (cell.cell_type === 'code' && !cell.output_area.trusted) { trusted = false; } - cell_array[i] = cell.toJSON(); + cell_array[i] = cell.toJSON(true); } var data = { cells: cell_array, @@ -2527,17 +2517,12 @@ define(function (require) { * Save this notebook on the server. This becomes a notebook instance's * .save_notebook method *after* the entire notebook has been loaded. * - * manual_save will be true if the save was manually trigered by the user */ - Notebook.prototype.save_notebook = function (check_last_modified, - manual_save) { + Notebook.prototype.save_notebook = function (check_last_modified) { if (check_last_modified === undefined) { check_last_modified = true; } - if (manual_save === undefined) { - manual_save = false; - } - + var error; if (!this._fully_loaded) { error = new Error("Load failed, save is disabled"); @@ -2553,13 +2538,6 @@ define(function (require) { // the notebook as needed. this.events.trigger('before_save.Notebook'); - // Garbage collect unused attachments. Only do this for manual save - // to avoid removing unused attachments while the user is editing if - // an autosave gets triggered in the midle of an edit - if (manual_save) { - this.remove_unused_attachments(); - } - // Create a JSON model to be sent to the server. var model = { type : "notebook", @@ -2743,7 +2721,7 @@ define(function (require) { var parent = utils.url_path_split(this.notebook_path)[0]; var p; if (this.dirty) { - p = this.save_notebook(true, true); + p = this.save_notebook(true); } else { p = Promise.resolve(); } @@ -3013,7 +2991,7 @@ define(function (require) { */ Notebook.prototype.save_checkpoint = function () { this._checkpoint_after_save = true; - this.save_notebook(true, true); + this.save_notebook(true); }; /** diff --git a/notebook/static/notebook/js/textcell.js b/notebook/static/notebook/js/textcell.js index b3c5dcf6c..c6772b9fc 100644 --- a/notebook/static/notebook/js/textcell.js +++ b/notebook/static/notebook/js/textcell.js @@ -123,44 +123,6 @@ define([ this.attachments[key][mime_type] = [b64_data]; }; - TextCell.prototype.remove_unused_attachments = function () { - // The general idea is to render the text, find attachment like when - // we substitute them in render() and mark used attachments by adding - // a temporary .used property to them. - if (Object.keys(this.attachments).length > 0) { - var that = this; - // To find unused attachments, rendering to HTML is easier than - // searching in the markdown source for the multiple ways you can - // reference an image in markdown (using []() or a HTML tag) - var text = this.get_text(); - marked(text, function (err, html) { - html = security.sanitize_html(html); - html = $($.parseHTML(html)); - html.find('img[src^="attachment:"]').each(function (i, h) { - h = $(h); - var key = h.attr('src').replace(/^attachment:/, ''); - if (key in that.attachments) { - that.attachments[key].used = true; - } - - // This is to avoid having the browser do a GET request - // on the invalid attachment: URL - h.attr('src', ''); - }); - }); - - for (var key in this.attachments) { - if (this.attachments[key].used === undefined) { - console.log('Dropping unused attachment ' + key); - delete this.attachments[key]; - } else { - // Remove temporary property - delete this.attachments[key].used; - } - } - } - } - TextCell.prototype.select = function () { var cont = Cell.prototype.select.apply(this, arguments); if (cont) { @@ -230,7 +192,6 @@ define([ */ TextCell.prototype.fromJSON = function (data) { Cell.prototype.fromJSON.apply(this, arguments); - console.log('data cell_type : ' + data.cell_type + ' this.cell_type : ' + this.cell_type); if (data.cell_type === this.cell_type) { if (data.attachments !== undefined) { this.attachments = data.attachments; @@ -251,18 +212,52 @@ define([ }; /** Generate JSON from cell + * @param {bool} gc_attachments - If true, will remove unused attachments + * from the returned JSON * @return {object} cell data serialised to json */ - TextCell.prototype.toJSON = function () { + TextCell.prototype.toJSON = function (gc_attachments) { + if (gc_attachments === undefined) { + gc_attachments = false; + } + var data = Cell.prototype.toJSON.apply(this); data.source = this.get_text(); if (data.source == this.placeholder) { data.source = ""; } - // Deepcopy the attachments so copied cells don't share the same + + // We deepcopy the attachments so copied cells don't share the same // objects if (Object.keys(this.attachments).length > 0) { - data.attachments = JSON.parse(JSON.stringify(this.attachments)); + if (gc_attachments) { + // Garbage collect unused attachments : The general idea is to + // render the text, and find used attachments like when we + // substitute them in render() + data.attachments = {}; + var that = this; + // To find attachments, rendering to HTML is easier than + // searching in the markdown source for the multiple ways you + // can reference an image in markdown (using []() or a + // HTML ) + var text = this.get_text(); + marked(text, function (err, html) { + html = security.sanitize_html(html); + html = $($.parseHTML(html)); + html.find('img[src^="attachment:"]').each(function (i, h) { + h = $(h); + var key = h.attr('src').replace(/^attachment:/, ''); + if (key in that.attachments) { + data.attachments[key] = JSON.parse(JSON.stringify( + that.attachments[key])); + } + + // This is to avoid having the browser do a GET request + // on the invalid attachment: URL + h.attr('src', ''); + }); + }); + } } return data; }; diff --git a/notebook/tests/notebook/attachments.js b/notebook/tests/notebook/attachments.js index cbeff5308..63029a680 100644 --- a/notebook/tests/notebook/attachments.js +++ b/notebook/tests/notebook/attachments.js @@ -100,11 +100,59 @@ casper.notebook_test(function () { "both cells have the attachments"); }); + var nbname = 'attachments_test.ipynb'; + this.thenEvaluate(function(nbname) { + IPython.notebook.set_notebook_name(nbname); + }, {nbname:nbname}); + // -- Save the notebook. This should cause garbage collection for the // second cell (since we just pasted the attachments but there is no // markdown referencing them) - this.thenEvaluate(function() { + this.thenEvaluate(function(nbname) { + IPython._checkpoint_created = false; + require(['base/js/events'], function (events) { + events.on('checkpoint_created.Notebook', function (evt, data) { + IPython._checkpoint_created = true; + }); + }); + IPython.notebook.save_checkpoint(); + }, {nbname:nbname}); + + this.waitFor(function () { + return this.evaluate(function(){ + return IPython._checkpoint_created; + }); + }); + + this.then(function(){ + this.open_dashboard(); + }); + + + this.then(function(){ + var notebook_url = this.evaluate(function(nbname){ + var escaped_name = encodeURIComponent(nbname); + var return_this_thing = null; + $("a.item_link").map(function (i,a) { + if (a.href.indexOf(escaped_name) >= 0) { + return_this_thing = a.href; + return; + } + }); + return return_this_thing; + }, {nbname:nbname}); + this.test.assertNotEquals(notebook_url, null, "Escaped URL in notebook list"); + // open the notebook + this.open(notebook_url); + }); + + // wait for the notebook + this.waitFor(this.kernel_running); + this.waitFor(function() { + return this.evaluate(function () { + return IPython && IPython.notebook && true; + }); }); this.then(function() {