Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#13351 closed Bug (fixed)

Linked image removes parent A after editing

Reported by: Alfonso Martínez de Lizarrondo Owned by: Szymon Cofalik
Priority: Normal Milestone: CKEditor 4.4.8
Component: General Version: 4.0
Keywords: Cc:

Description

Check http://jsfiddle.net/fbpk5sh3/

If the editor is configured to hide the link tab in the image dialog (just to avoid clutter), then editing the image (even just pressing OK) removes the parent A element.

Double click the image in the first editor, press OK, check that it's still linked.

Repeat with the second editor, the parent A is removed as shown by the image border and status bar.

It's a silent bug that destroys the content.

Change History (10)

comment:1 Changed 3 years ago by Piotrek Koszuliński

Can't check right now, but it sounds like related to #12739. If I'm right, it may be already fixed on master.

comment:2 Changed 3 years ago by Alfonso Martínez de Lizarrondo

It's related in the sense that the problem is related to a removed tab in a dialog, but it's not fixed (at least by that ticket) because this problem is in the Image dialog and that one on the Link dialog (and that one talked about advanced attributes, this one the container link)

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

Status: newconfirmed
Version: 4.4.7

comment:4 Changed 3 years ago by Szymon Cofalik

Owner: set to Szymon Cofalik
Status: confirmedassigned

comment:5 Changed 3 years ago by Szymon Cofalik

Version: 4.0

comment:6 Changed 3 years ago by Szymon Cofalik

Status: assignedreview

Pushed branch:t/13351

this.addLink flag was always set to false when Link tab was disabled because it's state is changed by commit method of fields that are specified for this tab. Now the default behavior is to set this.addLink to true if there is a link on the image.

Last edited 3 years ago by Szymon Cofalik (previous) (diff)

comment:7 Changed 3 years ago by Piotrek Koszuliński

Status: reviewassigned

I rebased branch:t/13351 and pushed one small commit there.

The fix seems to be correct, but I wonder how this change affects creating an image. In dialog's onShow() callback I can see that we look for a link when something is selected. What if it isn't image what's selected, but some other element (so this dialog will insert an image rather than edit it)? I think it would make sense to test cases like:

bender.tools.selection.setWithHtml( editor, '<p>x[<a href="foo">foo</a>]x</p>' );
bender.tools.selection.setWithHtml( editor, '<p>x<a href="foo">y[<span contenteditable="false">foo</span>]y</a>x</p>' );

In this cases an element is selected (so getSelectedElement() should return something if I'm not mistaken) and link exists, but image is not being edited.

So what we need now are tests in tests/plugins/image/ for cases which may be affected. There are not many tests there, so you may need to add few more for cases that should not be affected (including these with link tab visible).

In tests/plugins/image/image.js you've got tests for the image dialog with the link tab visible, so it will be best to add tests for cases without this tab in a second file.

comment:8 Changed 3 years ago by Szymon Cofalik

Status: assignedreview

branch:t/13351

I have refactored old tests and added some missing tests (also connected with reported error). I have also fixed javascript crash when Image dialog was opened on

[<a href="#">foo</a>]
Last edited 3 years ago by Szymon Cofalik (previous) (diff)

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

Milestone: CKEditor 4.4.8
Resolution: fixed
Status: reviewclosed

Fixed on master with git:7e5367f.

I had to push few commits to the branch to address some minor issues. Please check them.

comment:10 Changed 3 years ago by Piotrek Koszuliński

Fixed also #12847.

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