Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#10729 closed Bug (invalid)

cke 4.2 selectall plugin breaks codemirror plugin

Reported by: Chris Cohen Owned by:
Priority: Normal Milestone:
Component: Core : Selection Version: 4.2
Keywords: Cc: chris.cohen@…

Description

Line 32 of selectall plugin needs to be changed to

if (editable.isVisible( )) editable.focus();

Change History (7)

comment:1 Changed 6 years ago by Chris Cohen

Cc: chris.cohen@… added
Component: GeneralCore : Selection
Keywords: selectall added
Version: 4.2

comment:2 Changed 6 years ago by Jakub Ś

Keywords: selectall removed
Resolution: invalid
Status: newclosed

cke 4.2 selectall plugin breaks codemirror plugin

  1. It isn't CKEditor that needs to adjust to third-party plugins but third-party plugins to editor.
  2. Plugin http://ckeditor.com/addon/codemirror has no confirmed support for version 4.2.

You should rather inform plugin author about this.

I'm invalidating this ticket but in order to exclude any possibility that there is something broken in editor please explain exactly in step by step scenario what is happening and why this line needs to be changed. This is important as “please fix this line” is just not enough information.

comment:3 Changed 6 years ago by Chris Cohen

Plugin author has been informed, but I think this is a regression in selectall plugin.

On stepping through I can see that the codemirror correctly overrides the selectall button exec but when control returns to the selectall plugin it attempts to focus on an element that has been hidden and so an error is thrown.

comment:4 Changed 6 years ago by Piotrek Koszuliński

So the codemirror DOES NOT correctly override selectall if it allows to focus invisible element. CKEditor's code cannot foreseen every possible case produced by overriding its normal behaviours.

Conclusion: Codemirror plugin needs to be fixed.

comment:5 Changed 6 years ago by Chris Cohen

On further investigation it appears that the focus method:

			focus: function() {
				// [IE] Use instead "setActive" method to focus the editable if it belongs to
				// the host page document, to avoid bringing an unexpected scroll.
				this.$[ CKEDITOR.env.ie && this.getDocument().equals( CKEDITOR.document ) ? 'setActive' : 'focus' ]();

needs to be sensitive to IE version - 9 and 10 provoke 'Incorrect function' using 'setActive' whereas 'focus' is ok - versions 7 and 8 are ok with setActive.

comment:6 Changed 6 years ago by Piotrek Koszuliński

If you think that there's an issue in that code, please report a ticket which exemplifies it. This one was about a bug in Codemirror, so there's no example of correct usage of CKEditor which causes an issue.

comment:7 Changed 6 years ago by Jakub Ś

What @Reinmar means is that you should report new ticket which besides solution shows situation where things go wrong and for which this solution can be applied.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy