From 57556889b98a5d3ccff487deaa8f4deff4448ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Wytr=C4=99bowicz?= Date: Fri, 17 Apr 2020 09:57:54 +0200 Subject: [PATCH] Other: Remove `env.isEdge`. Closes ckeditor/ckeditor5#6202. Remove some special cases for Edge, as since it's Chromium-based now, it behaves closer to others. --- packages/ckeditor5-widget/src/utils.js | 23 +++------- packages/ckeditor5-widget/tests/utils.js | 43 ------------------- .../tests/widget-integration.js | 2 - packages/ckeditor5-widget/tests/widget.js | 4 -- 4 files changed, 7 insertions(+), 65 deletions(-) diff --git a/packages/ckeditor5-widget/src/utils.js b/packages/ckeditor5-widget/src/utils.js index b3e27dd63fe..d7526462582 100644 --- a/packages/ckeditor5-widget/src/utils.js +++ b/packages/ckeditor5-widget/src/utils.js @@ -9,7 +9,6 @@ import HighlightStack from './highlightstack'; import IconView from '@ckeditor/ckeditor5-ui/src/icon/iconview'; -import env from '@ckeditor/ckeditor5-utils/src/env'; import dragHandleIcon from '../theme/icons/drag-handle.svg'; @@ -90,11 +89,7 @@ export function isWidget( node ) { */ /* eslint-enable max-len */ export function toWidget( element, writer, options = {} ) { - // The selection on Edge behaves better when the whole editor contents is in a single contenteditable element. - // https://github.com/ckeditor/ckeditor5/issues/1079 - if ( !env.isEdge ) { - writer.setAttribute( 'contenteditable', 'false', element ); - } + writer.setAttribute( 'contenteditable', 'false', element ); writer.addClass( WIDGET_CLASS_NAME, element ); writer.setCustomProperty( 'widget', true, element ); @@ -220,17 +215,13 @@ export function getLabel( element ) { export function toWidgetEditable( editable, writer ) { writer.addClass( [ 'ck-editor__editable', 'ck-editor__nested-editable' ], 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 ); + // Set initial contenteditable value. + writer.setAttribute( 'contenteditable', editable.isReadOnly ? 'false' : 'true', editable ); - // Bind the 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 ) { diff --git a/packages/ckeditor5-widget/tests/utils.js b/packages/ckeditor5-widget/tests/utils.js index 04ebd065aa9..8e0c7495cb6 100644 --- a/packages/ckeditor5-widget/tests/utils.js +++ b/packages/ckeditor5-widget/tests/utils.js @@ -23,7 +23,6 @@ import { WIDGET_CLASS_NAME } from '../src/utils'; import UIElement from '@ckeditor/ckeditor5-engine/src/view/uielement'; -import env from '@ckeditor/ckeditor5-utils/src/env'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import Model from '@ckeditor/ckeditor5-engine/src/model/model'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; @@ -37,9 +36,6 @@ describe( 'widget utils', () => { testUtils.createSinonSandbox(); beforeEach( () => { - // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. - testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); - viewDocument = new ViewDocument(); writer = new DowncastWriter( viewDocument ); @@ -126,19 +122,6 @@ describe( 'widget utils', () => { expect( icon.classList.contains( 'ck' ) ).to.be.true; expect( icon.classList.contains( 'ck-icon' ) ).to.be.true; } ); - - describe( 'on Edge', () => { - beforeEach( () => { - testUtils.sinon.stub( env, 'isEdge' ).get( () => true ); - - element = writer.createContainerElement( 'div' ); - toWidget( element, writer ); - } ); - - it( 'should not set contenteditable onEdge', () => { - expect( element.getAttribute( 'contenteditable' ) ).to.be.undefined; - } ); - } ); } ); describe( 'isWidget()', () => { @@ -220,32 +203,6 @@ describe( 'widget utils', () => { element.isFocused = false; expect( element.hasClass( 'ck-editor__nested-editable_focused' ) ).to.be.false; } ); - - describe( 'on Edge', () => { - beforeEach( () => { - testUtils.sinon.stub( env, 'isEdge' ).get( () => true ); - - viewDocument = new ViewDocument(); - element = new ViewEditableElement( viewDocument, 'div' ); - toWidgetEditable( element, writer ); - } ); - - it( 'should add contenteditable attribute when element is read-only - initialization', () => { - const element = new ViewEditableElement( viewDocument, 'div' ); - 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()', () => { diff --git a/packages/ckeditor5-widget/tests/widget-integration.js b/packages/ckeditor5-widget/tests/widget-integration.js index dbabca22683..f2f9e95f54d 100644 --- a/packages/ckeditor5-widget/tests/widget-integration.js +++ b/packages/ckeditor5-widget/tests/widget-integration.js @@ -26,8 +26,6 @@ describe( 'Widget - integration', () => { testUtils.createSinonSandbox(); beforeEach( () => { - // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. - testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); testUtils.sinon.stub( env, 'isSafari' ).get( () => true ); editorElement = document.createElement( 'div' ); diff --git a/packages/ckeditor5-widget/tests/widget.js b/packages/ckeditor5-widget/tests/widget.js index 64970a71498..10440467685 100644 --- a/packages/ckeditor5-widget/tests/widget.js +++ b/packages/ckeditor5-widget/tests/widget.js @@ -12,7 +12,6 @@ import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventd import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; -import env from '@ckeditor/ckeditor5-utils/src/env'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; /* global document */ @@ -23,9 +22,6 @@ describe( 'Widget', () => { testUtils.createSinonSandbox(); beforeEach( () => { - // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. - testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); - return VirtualTestEditor.create( { plugins: [ Widget, Typing ] } ) .then( newEditor => { editor = newEditor;