8
0
Quellcode durchsuchen

Merge pull request #46 from ckeditor/t/ckeditor5/1079

Other: Don't set `contenteditable` property for widgets and their nested editables on Edge due to awful instability which it causes in this browser. Closes ckeditor/ckeditor5#1079. Closes ckeditor/ckeditor5#1067.
Piotrek Koszuliński vor 7 Jahren
Ursprung
Commit
55a84030ac
2 geänderte Dateien mit 68 neuen und 7 gelöschten Zeilen
  1. 17 7
      packages/ckeditor5-widget/src/utils.js
  2. 51 0
      packages/ckeditor5-widget/tests/utils.js

+ 17 - 7
packages/ckeditor5-widget/src/utils.js

@@ -10,6 +10,7 @@
 import HighlightStack from './highlightstack';
 import Position from '@ckeditor/ckeditor5-engine/src/view/position';
 import IconView from '@ckeditor/ckeditor5-ui/src/icon/iconview';
+import env from '@ckeditor/ckeditor5-utils/src/env';
 
 import dragHandlerIcon from '../theme/icons/drag-handler.svg';
 
@@ -57,7 +58,12 @@ export function isWidget( element ) {
  * @returns {module:engine/view/element~Element} Returns same element.
  */
 export function toWidget( element, writer, options = {} ) {
-	writer.setAttribute( 'contenteditable', 'false', element );
+	// The selection on Edge behaves better when the whole editor contents is in a single contentedible element.
+	// https://github.com/ckeditor/ckeditor5/issues/1079
+	if ( !env.isEdge ) {
+		writer.setAttribute( 'contenteditable', 'false', element );
+	}
+
 	writer.addClass( WIDGET_CLASS_NAME, element );
 	writer.setCustomProperty( widgetSymbol, true, element );
 	element.getFillerOffset = getFillerOffset;
@@ -154,13 +160,17 @@ export function getLabel( element ) {
 export function toWidgetEditable( editable, writer ) {
 	writer.addClass( [ 'ck-editor__editable', 'ck-editor__nested-editable' ], editable );
 
-	// Set initial contenteditable value.
-	writer.setAttribute( 'contenteditable', editable.isReadOnly ? 'false' : 'true', editable );
+	// The selection on Edge behaves better when the whole editor contents is in a single contentedible element.
+	// https://github.com/ckeditor/ckeditor5/issues/1079
+	if ( !env.isEdge ) {
+		// Set initial contenteditable value.
+		writer.setAttribute( 'contenteditable', editable.isReadOnly ? 'false' : 'true', editable );
 
-	// Bind contenteditable property to element#isReadOnly.
-	editable.on( 'change:isReadOnly', ( evt, property, is ) => {
-		writer.setAttribute( 'contenteditable', is ? 'false' : 'true', editable );
-	} );
+		// Bind the contenteditable property to element#isReadOnly.
+		editable.on( 'change:isReadOnly', ( evt, property, is ) => {
+			writer.setAttribute( 'contenteditable', is ? 'false' : 'true', editable );
+		} );
+	}
 
 	editable.on( 'change:isFocused', ( evt, property, is ) => {
 		if ( is ) {

+ 51 - 0
packages/ckeditor5-widget/tests/utils.js

@@ -19,11 +19,17 @@ import {
 	WIDGET_CLASS_NAME
 } from '../src/utils';
 import UIElement from '@ckeditor/ckeditor5-engine/src/view/uielement';
+import env from '@ckeditor/ckeditor5-utils/src/env';
 
 describe( 'widget utils', () => {
+	const initialEnvEdge = env.isEdge;
+
 	let element, writer, viewDocument;
 
 	beforeEach( () => {
+		// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+		env.isEdge = false;
+
 		viewDocument = new ViewDocument();
 		writer = new Writer( viewDocument );
 
@@ -31,6 +37,10 @@ describe( 'widget utils', () => {
 		toWidget( element, writer );
 	} );
 
+	afterEach( () => {
+		env.isEdge = initialEnvEdge;
+	} );
+
 	describe( 'toWidget()', () => {
 		it( 'should set contenteditable to "false"', () => {
 			expect( element.getAttribute( 'contenteditable' ) ).to.equal( 'false' );
@@ -110,6 +120,19 @@ describe( 'widget utils', () => {
 			expect( icon.classList.contains( 'ck' ) ).to.be.true;
 			expect( icon.classList.contains( 'ck-icon' ) ).to.be.true;
 		} );
+
+		describe( 'on Edge', () => {
+			beforeEach( () => {
+				env.isEdge = true;
+
+				element = writer.createContainerElement( 'div' );
+				toWidget( element, writer );
+			} );
+
+			it( 'should not set contenteditable onEdge', () => {
+				expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;
+			} );
+		} );
 	} );
 
 	describe( 'isWidget()', () => {
@@ -189,6 +212,34 @@ describe( 'widget utils', () => {
 			element.isFocused = false;
 			expect( element.hasClass( 'ck-editor__nested-editable_focused' ) ).to.be.false;
 		} );
+
+		describe( 'on Edge', () => {
+			beforeEach( () => {
+				env.isEdge = true;
+
+				viewDocument = new ViewDocument();
+				element = new ViewEditableElement( 'div' );
+				element._document = viewDocument;
+				toWidgetEditable( element, writer );
+			} );
+
+			it( 'should add contenteditable attribute when element is read-only - initialization', () => {
+				const element = new ViewEditableElement( 'div' );
+				element._document = viewDocument;
+				element.isReadOnly = true;
+				toWidgetEditable( element, writer );
+
+				expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;
+			} );
+
+			it( 'should add contenteditable attribute when element is read-only - when changing', () => {
+				element.isReadOnly = true;
+				expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;
+
+				element.isReadOnly = false;
+				expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined;
+			} );
+		} );
 	} );
 
 	describe( 'addHighlightHandling()', () => {