Skip to content

attachment mimebundle values are strings, not lists #2159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 8, 2017
Merged

attachment mimebundle values are strings, not lists #2159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f982c0d
attachment mimebundle values are strings, not lists
minrk Feb 8, 2017
f638807
copying cells copies attachments
minrk Feb 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions notebook/static/notebook/js/textcell.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ define([
* Add a new attachment to this cell
*/
this.attachments[key] = {};
this.attachments[key][mime_type] = [b64_data];
this.attachments[key][mime_type] = b64_data;
};

TextCell.prototype.select = function () {
Expand Down Expand Up @@ -208,7 +208,7 @@ define([
// to this state, instead of a blank cell
this.code_mirror.clearHistory();
// TODO: This HTML needs to be treated as potentially dangerous
// user input and should be handled before set_rendered.
// user input and should be handled before set_rendered.
this.set_rendered(data.rendered || '');
this.rendered = false;
this.render();
Expand Down Expand Up @@ -239,8 +239,8 @@ define([
// 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;
data.attachments = {};
// 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
Expand All @@ -252,7 +252,7 @@ define([
html.find('img[src^="attachment:"]').each(function (i, h) {
h = $(h);
var key = h.attr('src').replace(/^attachment:/, '');
if (key in that.attachments) {
if (that.attachments.hasOwnProperty(key)) {
data.attachments[key] = JSON.parse(JSON.stringify(
that.attachments[key]));
}
Expand All @@ -262,6 +262,12 @@ define([
h.attr('src', '');
});
});
if (data.attachments.length === 0) {
// omit attachments dict if no attachments
delete data.attachments;
}
} else {
data.attachments = JSON.parse(JSON.stringify(this.attachments));
}
}
return data;
Expand Down Expand Up @@ -343,7 +349,7 @@ define([
*/
var that = this;
var pos = this.code_mirror.getCursor();
var reader = new FileReader;
var reader = new FileReader();
// We can get either a named file (drag'n'drop) or a blob (copy/paste)
// We generate names for blobs
var key;
Expand Down Expand Up @@ -412,10 +418,10 @@ define([
h = $(h);
var key = h.attr('src').replace(/^attachment:/, '');

if (key in that.attachments) {
if (that.attachments.hasOwnProperty(key)) {
var att = that.attachments[key];
var mime = Object.keys(att)[0];
h.attr('src', 'data:' + mime + ';base64,' + att[mime][0]);
h.attr('src', 'data:' + mime + ';base64,' + att[mime]);
} else {
h.attr('src', '');
}
Expand Down
55 changes: 38 additions & 17 deletions notebook/tests/notebook/attachments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ casper.notebook_test(function () {
var selector = '#insert_image > a';
this.waitForSelector(selector);
this.thenEvaluate(function(sel) {
IPython.notebook.to_markdown();
var cell = IPython.notebook.get_selected_cell();
Jupyter.notebook.to_markdown();
var cell = Jupyter.notebook.get_selected_cell();
cell.set_text("");
cell.unrender();

Expand Down Expand Up @@ -43,14 +43,14 @@ casper.notebook_test(function () {
// Validate and render the markdown cell
this.thenClick('#btn_ok');
this.thenEvaluate(function() {
IPython.notebook.get_cell(0).render();
Jupyter.notebook.get_cell(0).render();
});
this.wait(300);
// Check that an <img> tag has been inserted and that it contains the
// image
this.then(function() {
var img = this.evaluate(function() {
var cell = IPython.notebook.get_cell(0);
var cell = Jupyter.notebook.get_cell(0);
var img = $("div.text_cell_render").find("img");
return {
src: img.attr("src"),
Expand Down Expand Up @@ -78,7 +78,7 @@ casper.notebook_test(function () {
// append a new cell
this.append_cell('', 'markdown');
this.thenEvaluate(function() {
IPython.notebook.select_next();
Jupyter.notebook.select_next();
});

// and paste the attachments into it
Expand All @@ -89,39 +89,60 @@ casper.notebook_test(function () {
// check that the new cell has attachments
this.then(function() {
var cell_attachments = this.evaluate(function() {
return IPython.notebook.get_selected_cell().attachments;
return Jupyter.notebook.get_selected_cell().attachments;
});
var orig_cell_attachments = this.evaluate(function() {
return IPython.notebook.get_cell(0).attachments;
return Jupyter.notebook.get_cell(0).attachments;
});
var clip = this.evaluate(function() { return IPython.notebook.clipboard_attachments; });
// Check that the two cells have the same attachments
this.test.assertEquals(cell_attachments, orig_cell_attachments,
"both cells have the attachments");
"pasted attachments ok");
});

// copy/paste cell includes attachments
selector = '#copy_cell > a';
this.waitForSelector(selector);
this.thenClick(selector);

selector = '#paste_cell_below > a';
this.waitForSelector(selector);
this.thenClick(selector);

// check that the new cell has attachments
this.then(function() {
var cell_attachments = this.evaluate(function() {
return Jupyter.notebook.get_selected_cell().attachments;
});
var orig_cell_attachments = this.evaluate(function() {
return Jupyter.notebook.get_cell(0).attachments;
});
// Check that the two cells have the same attachments
this.test.assertEquals(cell_attachments, orig_cell_attachments,
"pasted cell has attachments");
});

var nbname = 'attachments_test.ipynb';
this.thenEvaluate(function(nbname) {
IPython.notebook.set_notebook_name(nbname);
Jupyter.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(nbname) {
IPython._checkpoint_created = false;
Jupyter._checkpoint_created = false;
require(['base/js/events'], function (events) {
events.on('checkpoint_created.Notebook', function (evt, data) {
IPython._checkpoint_created = true;
Jupyter._checkpoint_created = true;
});
});

IPython.notebook.save_checkpoint();
Jupyter.notebook.save_checkpoint();
}, {nbname:nbname});

this.waitFor(function () {
return this.evaluate(function(){
return IPython._checkpoint_created;
return Jupyter._checkpoint_created;
});
});

Expand Down Expand Up @@ -151,16 +172,16 @@ casper.notebook_test(function () {
this.waitFor(this.kernel_running);
this.waitFor(function() {
return this.evaluate(function () {
return IPython && IPython.notebook && true;
return Jupyter && Jupyter.notebook && true;
});
});

this.then(function() {
var cell0 = this.evaluate(function() {
return IPython.notebook.get_cell(0);
return Jupyter.notebook.get_cell(0);
});
var cell1 = this.evaluate(function() {
return IPython.notebook.get_cell(1);
return Jupyter.notebook.get_cell(1);
});
this.test.assert('black_square_22.png' in cell0.attachments,
'cell0 has kept its attachments');
Expand Down