Image dialog: pre-filling 'width' and 'height' should be configurable

[Martin Dietze]

In some cases users do not want hard-set image dimensions in the generated IMG tags by default, and clearing those fields each time an image gets added is error-prone and annoying. Thus this behavior should be configurable.

The attached patch is a three-liner that adds a configuration option 'editor.config.image_emptyHeightWidth' that can be set to 'false' to suppress the default.

Edit

PS. We need options for image2 as well.

[Martin Dietze]

Patch adding a 'editor.config.image_emptyHeightWidth' option to suppress pre-filling image dimensions in dialog.

[Jakub Ś]

This should be made possible with below setting:

var editor = CKEDITOR.replace( 'editor1', {	
	disallowedContent : 'img{width, height}[width, height]'				
});

There is a problem however: #12132.

[Piotrek Koszuliński]

I'm not sure that these are exactly the same solutions - ACF will disable inputs, when the ticket says about not filling inputs automatically.

Additionally, this ticket could be generalised, because image2 also suffers.

[Piotrek Koszuliński]

I'm setting milestone to 4.5. The reason is that this option is ultra small and it cannot be done without changing CKEditor source.

[Martin Dietze]

As release 4.5 is scheduled for next week and I haven't observed any change to this ticket, please forgive me for asking again - is my patch going to be included in 4.5?

[Piotrek Koszuliński]

Replying to mbert:

As release 4.5 is scheduled for next week and I haven't observed any change to this ticket, please forgive me for asking again - is my patch going to be included in 4.5?

4.5 will not be released according to schedule. We needed to change our priorities and we haven't updated the ETA yet (mostly, because we don't know enough).

[Artur Delura]

At least changes will be mastered soon hopefully. Changes in ​branch:t/12126.

[Piotrek Koszuliński]

I think that the configuration option should be called "_prefillDimensions" or "_prefillSize". WDYT, Ania?

[Piotrek Koszuliński]

BTW. See ​http://dev.ckeditor.com/ticket/12615#comment:9

[Piotr Jasiun]

• you introduced additional parameters widthValue and heightValue in resetSize ​https://github.com/cksource/ckeditor-dev/blob/cbb1073f1e431ef27ac067cb36c83127081e55a3/plugins/image/dialogs/image.js#L153 to call then only once with the value 0 and 0. ​https://github.com/cksource/ckeditor-dev/blob/cbb1073f1e431ef27ac067cb36c83127081e55a3/plugins/image/dialogs/image.js#L225 It woulde be cleaner to pass editor.config.image_emptyDimensionsOnLoad as a parameter instead of widthValue and heightValue,

• is not it better to call setValue only if image2_emptyDimensionsOnLoad is not true instead of calling setValue with value 0? ​https://github.com/cksource/ckeditor-dev/blob/cbb1073f1e431ef27ac067cb36c83127081e55a3/plugins/image2/dialogs/image2.js#L148-L152
• change _emptyDimensionsOnLoad to _prefillDimensions.

[Piotr Jasiun]

Also, because this ticket introduce new configuration option, it should have manual test with prepared editor so it might be tested easily before release. It might be a general manual test.

[Artur Delura]

Replying to pjasiun:

• you introduced additional parameters widthValue and heightValue in resetSize ​https://github.com/cksource/ckeditor-dev/blob/cbb1073f1e431ef27ac067cb36c83127081e55a3/plugins/image/dialogs/image.js#L153 to call then only once with the value 0 and 0. ​https://github.com/cksource/ckeditor-dev/blob/cbb1073f1e431ef27ac067cb36c83127081e55a3/plugins/image/dialogs/image.js#L225 It woulde be cleaner to pass editor.config.image_emptyDimensionsOnLoad as a parameter instead of widthValue and heightValue,

Good point change is ​here.

• is not it better to call setValue only if image2_emptyDimensionsOnLoad is not true instead of calling setValue with value 0? ​https://github.com/cksource/ckeditor-dev/blob/cbb1073f1e431ef27ac067cb36c83127081e55a3/plugins/image2/dialogs/image2.js#L148-L152

Unfortunatelly we can't do this, because we have to empty input values when we change image src and focusout. See second test ​here and ​here.

• change _emptyDimensionsOnLoad to _prefillDimensions.

Done.

[Artur Delura]

Replying to pjasiun:

Also, because this ticket introduce new configuration option, it should have manual test with prepared editor so it might be tested easily before release. It might be a general manual test.

Done.

[Artur Delura]

Changes in ​branch:t/12126.

[Piotr Jasiun]

I have rebased branch on the newest major and pushed some changes.

The most important is test fix. plugins/image2/editing#test dimension should be empty after resetting size and loading image was red when I run all tests in the test suit on Chrome. This was because you did not add unique string at the end of the image URL so, if the previews test already downloaded that image, in was taken from cache. Because of that image in tests was downloaded before the real image and assertion failed.

Changes are ready to be majorized. Please check them and majorise if you have no remarks.

[Artur Delura]

Majorized in ​git:df5a9dc.