Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In ckeditor, Scroll bar returns to top when changing character color in table and setting alignment to left or right . #4248

Open
sayali114shinde opened this issue Sep 2, 2020 · 8 comments
Labels
browser:firefox The issue can only be reproduced in the Firefox browser. plugin:table The plugin which probably causes the issue. plugin:tabletools The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.

Comments

@sayali114shinde
Copy link

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Create 10*2 table
  2. insert content in every cell such that Scroll bar appears.
  3. Scroll down and Go to last cell.
  4. Change the text color of cell.
  5. Right click on table and change alignment to Right.

Expected result

Scroll bar should keep its original position.

Actual result

Scroll bar cannot keep its original position and returns to the top.

Other details

  • Browser: Specific to FireFox
  • OS: Win 10
  • CKEditor version: 4.13.1
  • Installed CKEditor plugins: Tested on ckeditor 4 Demo

ckeditor_TableRightAlignment-Issue

@sayali114shinde sayali114shinde added the type:bug A bug. label Sep 2, 2020
@sayali114shinde sayali114shinde changed the title scroll bar returns to top when setting alignment to left or right and changing character color in table. In ckeditor, Scroll bar returns to top when changing character color in table and setting alignment to left or right . Sep 2, 2020
@Comandeer Comandeer added browser:firefox The issue can only be reproduced in the Firefox browser. plugin:table The plugin which probably causes the issue. plugin:tabletools The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. labels Sep 6, 2020
@Comandeer
Copy link
Member

I can confirm the issue.

@sayali114shinde
Copy link
Author

We are waiting for your support to resolve this asap.

@f1ames
Copy link
Contributor

f1ames commented Sep 25, 2020

@sayali114shinde we don't have any ETA for this issue due to other priorities. If you have CKEditor license, please contact our support team regarding this issue.

@NavnathKumbhar
Copy link

Any update on this please?

@Dumluregn
Copy link
Contributor

Hello, sorry but no change regarding this issue on our side.

@NavnathKumbhar
Copy link

NavnathKumbhar commented Feb 16, 2021

Hello,

I have explored this issue more.
It is not only about table alignment and color change. When we change paragraph formatting, styles, fonts etc. , in that case also, scroll jumps up to the editing area.
Also, I found this issue for Image plugin also.
I confirm that this issue is for Firefox browser only.

After some exploration, I found that CKEditor editor.focus() API is causing the scroll to jump up to the top.
I tried to comment out this function call at below places:

  1. Inside CKEDITOR.command function:

this.exec = function( data ) {
if ( this.state == CKEDITOR.TRISTATE_DISABLED || !this.checkAllowed() )
return false;

  //if ( this.editorFocus ) // Give editor focus if necessary (https://dev.ckeditor.com/ticket/4355).
  	//editor.focus();

  if ( this.fire( 'exec' ) === false )
  	return true;

  return ( commandDefinition.exec.call( this, editor, data ) !== false );

}

  1. Inside dialogbox hide event:
  	// Deduct or clear the z-index.
  	if ( !this._.parentDialog ) {
  		CKEDITOR.dialog._.currentZIndex = null;

  		// Remove access key handlers.
  		element.removeListener( 'keydown', accessKeyDownHandler );
  		element.removeListener( 'keyup', accessKeyUpHandler );

  		var editor = this._.editor;
  		//editor.focus(); 

  		// Give a while before unlock, waiting for focus to return to the editable. (https://dev.ckeditor.com/ticket/172)
  		setTimeout( function() {
  			editor.focusManager.unlock();

  			// Fixed iOS focus issue (https://dev.ckeditor.com/ticket/12381).
  			// Keep in mind that editor.focus() does not work in this case.
  			if ( CKEDITOR.env.iOS ) {
  				editor.window.focus();
  			}
  		}, 0 );

  	} 
  1. Inside "colorbutton" plugin => addButton function:
  		editor.addCommand( commandName, {
  			contextSensitive: true,
  			exec: function( editor, data ) {
  				if ( editor.readOnly ) {
  					return;
  				}

  				var newStyle = data.newStyle;

  				editor.removeStyle( defaultColorStyle );

  				//editor.focus();

  				if ( newStyle ) {
  					editor.applyStyle( newStyle );
  				}

  				editor.fire( 'saveSnapshot' );
  			}
  1. Inside "colorbutton" plugin => createClickFunction function:
  		function createClickFunction() {
  			return CKEDITOR.tools.addFunction( function addClickFn( color, colorName, colorbox ) {
  				//editor.focus();
  				editor.fire( 'saveSnapshot' );

  				if ( color == '?' ) {
  					editor.getColorFromDialog( function( color ) {
  						if ( color ) {
  							setColor( color, colorName, history );
  						}
  					}, null, colorData );
  				} else {
  					setColor( color && '#' + color, colorName, history );
  				}

  				// The colors may be duplicated in both default palette and color history. If user reopens panel
  				// after choosing color without changing selection, the box that was clicked last should be selected.
  				// If selection changes in the meantime, color box from the default palette has the precedence.
  				// See https://github.com/ckeditor/ckeditor4/pull/3784#pullrequestreview-378461341 for details.
  				if ( !colorbox ) {
  					return;
  				}
  				colorbox.setAttribute( 'cke_colorlast', true );
  				editor.once( 'selectionChange', function() {
  					colorbox.removeAttribute( 'cke_colorlast' );
  				} );
  			} );
  		}
  1. Inside "font" change event handler:
  function onClickHandler( newValue ) {
  	var oldValue = this.getValue();

  	//editor.focus();

  	editor.fire( 'saveSnapshot' );

  	editor.execCommand( definition.commandName, {
  		newStyle: stylesData.getStyle( newValue ),
  		oldStyle: stylesData.getStyle( oldValue )
  	} );

  	editor.fire( 'saveSnapshot' );
  }
  1. Inside paragraph formatting combo onClick function:
  	onClick: function( value ) {
  		//editor.focus();
  		editor.fire( 'saveSnapshot' );

  		var style = styles[ value ],
  			elementPath = editor.elementPath();

  		// (#3649)
  		editor.fire( 'stylesRemove', { type: CKEDITOR.STYLE_BLOCK } );

  		// Always apply style, do not allow to toggle it by clicking on corresponding list item (#584).
  		if ( !style.checkActive( elementPath, editor ) ) {
  			editor.applyStyle( style );
  		}

  		// Save the undo snapshot after all changes are affected. (https://dev.ckeditor.com/ticket/4899)
  		setTimeout( function() {
  			editor.fire( 'saveSnapshot' );
  		}, 0 );
  	}
  1. Inside "Object Styles" onClick event:
  		onClick: function( value ) {
  			//editor.focus();
  			editor.fire( 'saveSnapshot' );

  			var style = styles[ value ],
  				elementPath = editor.elementPath();

  			// When more then one style from the same group is active ( which is not ok ),
  			// remove all other styles from this group and apply selected style.
  			if ( style.group && style.removeStylesFromSameGroup( editor ) ) {
  				editor.applyStyle( style );
  			} else {
  				editor[ style.checkActive( elementPath, editor ) ? 'removeStyle' : 'applyStyle' ]( style );
  			}

  			editor.fire( 'saveSnapshot' );
  		},

After commenting focus API at all these places, scroll does not jump when:

  1. I change the color
  2. Align the table/Image
  3. Change font, paragraph formatting, object styles.

But, as obvious, the regression here is that the cursor is not placed where it was before [In fact there is no cursor blinking in above scenarios]

Now, I know the culprit behind this issue but don't know the reason behind it.
Do you have any idea?

Or, If I found some alternative for this editor.focus() [Only for Firebox], will it be acceptable for code contribution against this issue?

For table alignment to work properly, I needed to append a wrapper around the table when we create it inside table.js , as below:

  // Insert the table element if we're creating one.
  if ( !this._.selectedElement ) {
     var wrapper = document.createElement('p');
                wrapper.style.width = "100%";
                wrapper.style.float = "left"; 
                wrapper.setAttribute("id","tableWrapper");
                var p = new CKEDITOR.dom.element(wrapper, editor.document);                        
                table.$.setAttribute("id","myTable");
                table.appendTo(p);
                editor.insertElement(p); 					
  			//editor.insertElement( table );
  			// Override the default cursor position after insertElement to place
  			// cursor inside the first cell (https://dev.ckeditor.com/ticket/7959), IE needs a while.
  			setTimeout( function() {
  				var firstCell = new CKEDITOR.dom.element( table.$.rows[ 0 ].cells[ 0 ] );
  				var range = editor.createRange();
  				range.moveToPosition( firstCell, CKEDITOR.POSITION_AFTER_START );
  				range.select();
  			}, 0 );
  		}

Thank you in advance for your feedback.

@Dumluregn
Copy link
Contributor

Hello, you are very welcome to create a pull request if you manage to find a solution to this problem. Just make sure to read our Contribution Guide.

@newpen
Copy link

newpen commented Jun 19, 2022

Any progress?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:firefox The issue can only be reproduced in the Firefox browser. plugin:table The plugin which probably causes the issue. plugin:tabletools The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

6 participants