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.

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 ) {

tests/utils.js

@@ -19,18 +19,28 @@ 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 );
 
  element = writer.createContainerElement( 'div' );
  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()', () => {