Opened 4 years ago

Closed 4 years ago

#12126 closed New Feature (fixed)

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

Reported by: Martin Dietze Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version:
Keywords: Cc: mdietze@…, a.tomanek@…

Description (last modified by Piotrek Koszuliński)

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.

Attachments (1)

0001-Add-new-configuration-option-editor.config.image_emp.patch (1.3 KB) - added by Martin Dietze 4 years ago.
Patch adding a 'editor.config.image_emptyHeightWidth' option to suppress pre-filling image dimensions in dialog.

Download all attachments as: .zip

Change History (18)

Changed 4 years ago by Martin Dietze

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

comment:1 Changed 4 years ago by Martin Dietze

Cc: mdietze@… added

comment:2 Changed 4 years ago by Jakub Ś

Version: 4.4.2

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.

comment:3 Changed 4 years ago by 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.

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

Description: modified (diff)
Milestone: CKEditor 4.5.0
Status: newconfirmed

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.

comment:5 Changed 4 years ago by 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?

comment:6 Changed 4 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

comment:7 in reply to:  5 Changed 4 years ago by 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).

comment:8 Changed 4 years ago by Artur Delura

Status: assignedreview

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

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

Cc: a.tomanek@… added

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

comment:11 Changed 4 years ago by Piotr Jasiun

Status: reviewreview_failed

comment:12 Changed 4 years ago by 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.

comment:13 in reply to:  11 Changed 4 years ago by Artur Delura

Replying to pjasiun:

Good point change is here.

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.

comment:14 in reply to:  12 Changed 4 years ago by 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.

comment:15 Changed 4 years ago by Artur Delura

Status: review_failedreview

Changes in branch:t/12126.

comment:16 Changed 4 years ago by Piotr Jasiun

Status: reviewreview_passed

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.

comment:17 Changed 4 years ago by Artur Delura

Resolution: fixed
Status: review_passedclosed

Majorized in git:df5a9dc.

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