From 3c6cf11de963f08e46f8ef35137d9729d7efcb3e Mon Sep 17 00:00:00 2001 From: f1ames Date: Wed, 24 Aug 2016 13:23:54 +0200 Subject: [PATCH 1/5] Use focus instead of setActive for Edge browser. --- core/editable.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/editable.js b/core/editable.js index 3c9f8cd9cb0..8b1898d29b2 100644 --- a/core/editable.js +++ b/core/editable.js @@ -74,10 +74,10 @@ } } - // [IE] Use instead "setActive" method to focus the editable if it belongs to - // the host page document, to avoid bringing an unexpected scroll. + // [IE] Use instead "setActive" method to focus the editable if it belongs to the host page document, + // to avoid bringing an unexpected scroll. Edge (starting from 14.14393) does not support setActive. try { - this.$[ CKEDITOR.env.ie && this.getDocument().equals( CKEDITOR.document ) ? 'setActive' : 'focus' ](); + this.$[ CKEDITOR.env.ie && !CKEDITOR.env.edge && this.getDocument().equals( CKEDITOR.document ) ? 'setActive' : 'focus' ](); } catch ( e ) { // IE throws unspecified error when focusing editable after closing dialog opened on nested editable. if ( !CKEDITOR.env.ie ) From 00750cfacced55f9a4c11b5a8e5201d49fc6418a Mon Sep 17 00:00:00 2001 From: f1ames Date: Fri, 26 Aug 2016 12:49:31 +0200 Subject: [PATCH 2/5] Restore scrollTop value after first editor focus (prevents Edge scrolling editor on focus). --- core/editable.js | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/core/editable.js b/core/editable.js index 8b1898d29b2..00b6515e8db 100644 --- a/core/editable.js +++ b/core/editable.js @@ -74,8 +74,15 @@ } } + // [Edge] Starting from EdgeHTML 14.14393, it does not support `setActive`. We need to use focus which + // causes unexpected scroll. Store scrollTop value so it can be restored after focusing editor. + // Scroll only happens if the editor is focused for the first time. (#14825) + if ( CKEDITOR.env.edge && CKEDITOR.env.version > 14 && !this.hasFocus && this.getDocument().equals( CKEDITOR.document ) ) { + this.editor._.previousScrollTop = this.$.scrollTop; + } + // [IE] Use instead "setActive" method to focus the editable if it belongs to the host page document, - // to avoid bringing an unexpected scroll. Edge (starting from 14.14393) does not support setActive. + // to avoid bringing an unexpected scroll. try { this.$[ CKEDITOR.env.ie && !CKEDITOR.env.edge && this.getDocument().equals( CKEDITOR.document ) ? 'setActive' : 'focus' ](); } catch ( e ) { @@ -872,6 +879,23 @@ }, null, null, -1 ); } + // [Edge] This is the other part of the workaround for Edge which restores saved + // scrollTop value and removes listener which is not needed anymore. (#14825) + if ( CKEDITOR.env.edge && CKEDITOR.env.version > 14 ) { + + var fixScrollOnFocus = function() { + var editable = editor.editable(); + + if ( editor._.previousScrollTop != null && editable.getDocument().equals( CKEDITOR.document ) ) { + editable.$.scrollTop = editor._.previousScrollTop; + editor._.previousScrollTop = null; + this.removeListener( 'scroll', fixScrollOnFocus ); + } + }; + + this.on( 'scroll', fixScrollOnFocus ); + } + // Register to focus manager. editor.focusManager.add( this ); From ee4f04b0ccc7d681bbbc0cdec93ba0e6db0e9206 Mon Sep 17 00:00:00 2001 From: f1ames Date: Fri, 26 Aug 2016 12:50:25 +0200 Subject: [PATCH 3/5] Tests: unit tests for scrolling on focus in Edge. --- tests/tickets/14825/1.js | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 tests/tickets/14825/1.js diff --git a/tests/tickets/14825/1.js b/tests/tickets/14825/1.js new file mode 100644 index 00000000000..5eaaa597ae3 --- /dev/null +++ b/tests/tickets/14825/1.js @@ -0,0 +1,40 @@ +/* bender-tags: 14825 */ + +( function() { + 'use strict'; + + bender.editors = { + divarea: { + name: 'divarea', + startupData: '

Paragraph 1

Paragraph 2

Paragraph 3

Paragraph 4

Paragraph 5

Paragraph 6

', + config: { + extraPlugins: 'divarea', + width: '100px', + height: '100px' + } + } + }; + + bender.test( { + 'test scroll position after focusing scrolled editor': function() { + if ( !( CKEDITOR.env.edge && CKEDITOR.env.version > 14 ) ) { + assert.ignore(); + } + + var editor = this.editors.divarea, + editable = editor.editable(); + + assert.areSame( 0, editable.$.scrollTop, 'Initial scrollTop is 0.' ); + + editable.find( 'p' ).getItem( 3 ).scrollIntoView(); + + var currentScrollTop = editable.$.scrollTop; + assert.isTrue( currentScrollTop > 0, 'Editor was scrolled successfully.' ); + + editor.focus(); + wait( function() { + assert.areSame( currentScrollTop, editable.$.scrollTop, 'Editor scrollTop value did not change after focus.' ); + }, 100 ); + } + } ); +} )(); From fdd5b39c03ee03d6802da007fa6f75c13c7170ad Mon Sep 17 00:00:00 2001 From: f1ames Date: Fri, 26 Aug 2016 13:05:46 +0200 Subject: [PATCH 4/5] Use focus starting from Edge 14 version. --- core/editable.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/editable.js b/core/editable.js index 00b6515e8db..ebfc573036d 100644 --- a/core/editable.js +++ b/core/editable.js @@ -84,7 +84,11 @@ // [IE] Use instead "setActive" method to focus the editable if it belongs to the host page document, // to avoid bringing an unexpected scroll. try { - this.$[ CKEDITOR.env.ie && !CKEDITOR.env.edge && this.getDocument().equals( CKEDITOR.document ) ? 'setActive' : 'focus' ](); + if ( CKEDITOR.env.ie && !( CKEDITOR.env.edge && CKEDITOR.env.version > 14 ) && this.getDocument().equals( CKEDITOR.document ) ) { + this.$.setActive(); + } else { + this.$.focus(); + } } catch ( e ) { // IE throws unspecified error when focusing editable after closing dialog opened on nested editable. if ( !CKEDITOR.env.ie ) From 3c63a3673fa28c5fe926f0d80ee38c3f214972bf Mon Sep 17 00:00:00 2001 From: Comandeer Date: Fri, 26 Aug 2016 15:22:31 +0200 Subject: [PATCH 5/5] Changelog entry. --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index f28ad83eb9d..982a0da2a90 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,7 @@ Fixed Issues: * [#13548](http://dev.ckeditor.com/ticket/13548): [IE] Fixed: Clicking the [elements path](http://ckeditor.com/addon/elementspath) disables Cut and Copy icons. * [#13812](http://dev.ckeditor.com/ticket/13812): Fixed: When aborting file upload the placeholder for image is left. * [#14659](http://dev.ckeditor.com/ticket/14659): [Blink] Fixed: Content scrolled to the top after closing the dialog in a [`
`-based editor](http://ckeditor.com/addon/divarea). +* [#14825](http://dev.ckeditor.com/ticket/14825): [Edge] Fixed: Focusing editor causes unwanted scrolling due to dropped support of `setActive` method. ## CKEditor 4.5.10