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

editor.resize() doesn't work with CSS units #1883

Closed
engineering-this opened this issue Apr 11, 2018 · 2 comments
Closed

editor.resize() doesn't work with CSS units #1883

engineering-this opened this issue Apr 11, 2018 · 2 comments
Assignees
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. type:bug A bug.

Comments

@engineering-this
Copy link
Contributor

Are you reporting a feature request or a bug?

Bug

Provide detailed reproduction steps (if any)

  1. Open sample CKEditor.
  2. Open console, and type CKEDITOR.instances[ 'your-instance-name' ].resize( '500em', '500em' )

Expected result

Editor is resized.

Actual result

Nothing happens

Other details

According to docs you should be able to pass

a CSS size value with unit as an argument.

Note: None of other CSS are working.

  • Browser: Any
  • OS: Any
  • CKEditor version: 4.9.2
  • Installed CKEditor plugins: resize
@engineering-this engineering-this added type:bug A bug. status:confirmed An issue confirmed by the development team. labels Apr 11, 2018
@mlewand mlewand added good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. target:minor Any docs related issue that can be merged into a master or major branch. labels Apr 11, 2018
@ashrafsabrym
Copy link

Yes, I suffer from this issue too. It's caused by this line of CKEDITOR.editor.prototype.resize in /core/creators/themedui.js:

outer.setSize( 'width', width, true );

While the definition of CKEDITOR.dom.element.prototype.setSize in /core/dom/element.js is:

/**
	 * Sets the element size considering the box model.
	 *
	 * @param {'width'/'height'} type The dimension to set.
	 * @param {Number} size The length unit in px.
	 * @param {Boolean} isBorderBox Apply the size based on the border box model.
	 */
	CKEDITOR.dom.element.prototype.setSize = function( type, size, isBorderBox ) {
		if ( typeof size == 'number' ) {
			if ( isBorderBox && !( CKEDITOR.env.ie && CKEDITOR.env.quirks ) )
				size -= marginAndPaddingSize.call( this, type );

			this.setStyle( type, size + 'px' );
		}
	};

Clearly, it receives only numeric values, thus CSS unit values fail.

And for height, there are those lines in the same method CKEDITOR.editor.prototype.resize:

resultContentsHeight = Math.max( height - ( isContentHeight ? 0 : contentsOuterDelta ), 0 ),
			resultOuterHeight = ( isContentHeight ? height + contentsOuterDelta : height );

		contents.setStyle( 'height', resultContentsHeight + 'px' );

If fed with CSS unit value strings, they will involve string and number computations which result in NaN and the end result applied to the element height is "NaNpx", which is rejected.

@f1ames f1ames removed the target:minor Any docs related issue that can be merged into a master or major branch. label Nov 12, 2019
@hub33k hub33k self-assigned this Apr 21, 2020
@jacekbogdanski
Copy link
Member

@ashrafsabrym we fixed the issue but note that this method is supposed to only work with absolute CSS units. Docs have been also updated as they could give incorrect information about this feature.

Closed in #4010

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

6 participants