Fixes microsoft#44805: Should not be able to undo in readonly editor

alexdima · alexdima · commit e7bc65ad3e1d · 2018-03-01T16:38:25.000+01:00
diff --git a/src/vs/editor/common/controller/cursor.ts b/src/vs/editor/common/controller/cursor.ts
@@ -342,11 +342,6 @@ export class Cursor extends viewEvents.ViewEventEmitter implements ICursors {
 			return;
 		}
 
-		if (this._configuration.editor.readOnly) {
-			// Cannot execute when read only
-			return;
-		}
-
 		if (opResult.shouldPushStackElementBefore) {
 			this._model.pushStackElement();
 		}
@@ -458,6 +453,12 @@ export class Cursor extends viewEvents.ViewEventEmitter implements ICursors {
 			return;
 		}
 
+		if (this._configuration.editor.readOnly) {
+			// All the remaining handlers will try to edit the model,
+			// but we cannot edit when read only...
+			return;
+		}
+
 		const oldState = new CursorModelState(this._model, this);
 		let cursorChangeReason = CursorChangeReason.NotSet;
 
diff --git a/src/vs/editor/test/browser/controller/cursor.test.ts b/src/vs/editor/test/browser/controller/cursor.test.ts
@@ -1906,6 +1906,34 @@ suite('Editor Controller - Regression tests', () => {
 			assertCursor(cursor, new Selection(1, 6, 1, 6));
 		});
 	});
+
+	test('issue #44805: Should not be able to undo in readonly editor', () => {
+		let model = TextModel.createFromString(
+			[
+				''
+			].join('\n'),
+			{
+				defaultEOL: DefaultEndOfLine.LF,
+				detectIndentation: false,
+				insertSpaces: false,
+				tabSize: 4,
+				trimAutoWhitespace: true
+			}
+		);
+
+		withTestCodeEditor(null, { readOnly: true, model: model }, (editor, cursor) => {
+			model.pushEditOperations([new Selection(1, 1, 1, 1)], [{
+				range: new Range(1, 1, 1, 1),
+				text: 'Hello world!'
+			}], () => [new Selection(1, 1, 1, 1)]);
+			assert.equal(model.getValue(EndOfLinePreference.LF), 'Hello world!');
+
+			cursorCommand(cursor, H.Undo, {});
+			assert.equal(model.getValue(EndOfLinePreference.LF), 'Hello world!');
+		});
+
+		model.dispose();
+	});
 });
 
 suite('Editor Controller - Cursor Configuration', () => {
