The type of a non-editable cell can be changed

[raffaele-cappelli]

Description

The type of cells with "editable": false can be changed (e.g. from Code to Markdown or Raw).

Reproduce

In a new notebook, edit metadata adding

"editable": false

the cell cannot be edited, but it is possible to change its type from the toolbar.

Expected behavior

It should not be possible to change the type, as in jupyter notebook.

Context

Reproduced in a clean env (conda create -n jlab-test --override-channels --strict-channel-priority -c conda-forge -c nodefaults jupyterlab)

JupyterLab Version 3.1.13
Google Chrome | 93.0.4577.82
Windows 10 OS Version 2009
JavaScript | V8 9.3.345.19

I was NOT able to reproduce the problem in Jupyter notebook.

[trevorcampbell]

I will bump this issue and add one more additional concern: the cell toolbar (that comes up when you hover over a cell) also ignores metadata.

If I have a cell that is false for both editable and deletable, the "trash can" button in the cell toolbar will still delete the cell.

[firai]

I will bump this issue and add one more additional concern: the cell toolbar (that comes up when you hover over a cell) also ignores metadata.

If I have a cell that is false for both editable and deletable, the "trash can" button in the cell toolbar will still delete the cell.

Which version are you running? I just tested the built-in cell toolbars in both 3.4.8 and the latest master, and neither toolbars will delete the cell when deletable is false. Are you referring to the enhanced-cell-toolbar extension (which I don't have installed and which is in a different repo) by any chance? If you can reproduce this issue with the built-in cell toolbar in the latest version of JupyterLab, this might need to be filed as a separate issue?

[trevorcampbell]

@firai so, this was something I saw a student do in a class I'm teaching that uses JupyterLab for student work. Our teaching team did actually try to reproduce it afterward, but to no avail. So it's possible the student copied one of our read-only cells, which I suppose didn't copy the "read-only" metadata, and then trashed that one.

Anyway I think you're safe to ignore my comment. I will re-file as a new issue if it comes up again!

[nthiery]

Let me bump this ticket. Our beginning students frequently change cell types by accident, e.g. by inadvertently typing keyboard shortcuts. And then they don't understand why the examples that we provide them with in locked cells are broken. When this happens outside of the classroom, it is a pain to debug remotely their "your example does not work" calls for help.

As an alternative, one could imagine having a separate flag called, say, can_change_type. Which would be useful by itself for answer cells that students are supposed to edit, but without changing their type.

cc @brichet

[krassowski]

Of note a guard of similar nature (but for pending input) was added in #16032. To solve this issue, one would need to add a check for read-only status, similar to the check for pending input in:

jupyterlab/packages/notebook/src/actions.tsx

Lines 2731 to 2746 in c0050f2

	if (
	child.model.type === 'code' &&
	(child as CodeCell).outputArea.pendingInput
	) {
	translator = translator || nullTranslator;
	const trans = translator.load('jupyterlab');
	// Do not permit changing cell type when input is pending
	void showDialog({
	title: trans.__('Cell type not changed due to pending input'),
	body: trans.__(
	'The cell type has not been changed to avoid kernel deadlock as this cell has pending input! Submit your pending input and try again.'
	),
	buttons: [Dialog.okButton()]
	});
	return;
	}

One could also add a test, building upon the test case in:

jupyterlab/packages/notebook/test/actions.spec.ts

Lines 669 to 700 in c0050f2

	it('should not change cell type away from code cell when input is pending', async () => {
	let cell = widget.activeCell as CodeCell;
	expect(widget.activeCell).toBeInstanceOf(CodeCell);
	const inputRequested = signalToPromise(cell.outputArea.inputRequested);
	// Execute input request
	cell.model.sharedModel.setSource('input()');
	// Do not wait for completion yet: it will not complete until input is submitted
	const donePromise = NotebookActions.run(widget, sessionContext);
	// Wait for the input being requested from kernel
	const stdin = (await inputRequested)[1];
	// Try to change cell type
	NotebookActions.changeCellType(widget, 'raw');
	// Cell type should stay unchanged.
	expect(widget.activeCell).toBeInstanceOf(CodeCell);
	// Should show a dialog informing user why cell type could not be changed
	await waitForDialog();
	await acceptDialog();
	// Submit the input
	(stdin as Stdin).handleEvent(
	new KeyboardEvent('keydown', { key: 'Enter' })
	);
	await donePromise;
	// Try to change cell type again, it should work now
	NotebookActions.changeCellType(widget, 'raw');
	expect(widget.activeCell).toBeInstanceOf(RawCell);
	});