Updated caching width and height for image2 on src change #1587
Conversation
tests/plugins/image2/editing.js
Outdated
| @@ -74,6 +74,37 @@ | |||
| wait(); | |||
| }, | |||
|
|
|||
| // #1348 | |||
There was a problem hiding this comment.
Please remember about proper issues referencing (see https://docs.ckeditor.com/ckeditor4/latest/guide/dev_tests.html#tagging-tests). Here should be:
// (#1348)
| @@ -0,0 +1,17 @@ | |||
| @bender-tags: 4.9.0, bug, 1348 | |||
There was a problem hiding this comment.
Please update tag to 4.9.1.
| @@ -0,0 +1,17 @@ | |||
| @bender-tags: 4.9.0, bug, 1348 | |||
| @bender-ui: collapsed | |||
| @bender-ckeditor-plugins: wysiwygarea, sourcearea, toolbar, image2, link, undo | |||
There was a problem hiding this comment.
Remember to include only these plugins which are necessary to run the test, here you can go just with wysiwygarea, toolbar, image2.
| @bender-ui: collapsed | ||
| @bender-ckeditor-plugins: wysiwygarea, sourcearea, toolbar, image2, link, undo | ||
|
|
||
| 1. Double click on the image to popup dialog. |
There was a problem hiding this comment.
Maybe Double click on the image to open dialog.?
| @bender-ckeditor-plugins: wysiwygarea, sourcearea, toolbar, image2, link, undo | ||
|
|
||
| 1. Double click on the image to popup dialog. | ||
| 2. Change url of the image to `/tests//_assets/lena.jpg`. |
|
|
||
| 1. Double click on the image to popup dialog. | ||
| 2. Change url of the image to `/tests//_assets/lena.jpg`. | ||
| 3. Ensure that the image is locked. |
There was a problem hiding this comment.
Ensure that image aspect ratio is locked. ?
|
|
||
| ## Expected | ||
|
|
||
| The image has preserved aspect ratio. |
There was a problem hiding this comment.
I would go with The inserted image has... (so it is clear that image should be inserted). Same in Unexpected.
There was a problem hiding this comment.
The changes to the test are fine, looks good!
However, because of the merge commit (a0431c3) every commit before it, is doubled (e.g. f9474c0 and 8f7c151) - so instead of 4 commits there are 8, which doesn't look good;) Could you rebase cleaning up the commit hisotry (just run git rebase -i HEAD~12 and then you will have a possiblity to drop unnecessary commits).
jacekbogdanski
force-pushed
the
t/1348
branch
from
6d26183 to
d44bb64
Compare
|
This looks bad. I will wait until we meet F2F to talk about this rebase. |
|
It should be merged to |
|
For some reason we missed this ticket during last releases, I'll update the tags in tests and merge it to 4.10.1. |
What is the purpose of this pull request?
Bug fix.
Does your PR contain necessary tests?
All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.
This PR contains
What changes did you make?
Updated caching width and height for image2 on src change. Without this change aspect ratio calculation was based on old image width/height.
Closes #1348.