Opened 7 years ago

Closed 7 years ago

#3829 closed Bug (fixed)

Links do not get deleted correctly

Reported by: damo Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: IBM Confirmed Review+ Cc:

Description

To reproduce:

  1. Open sample
  2. Add a word
  3. Select the word and create a link from it using the link dialog
  4. Goto source to see the link is created and return to RT mode
  5. Select the new link and delete it.
  6. Goto source again, see that that label of the link is gone but the <a> is still there.

Expected result: After deleting the link the <a> should also be deleted.

Attachments (2)

DeleteLinkExample.png (44.4 KB) - added by damo 7 years ago.
3829.patch (2.6 KB) - added by garry.yao 7 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by garry.yao

  • Keywords WorksForMe added

Can you provide details about how the link is deleted in step 5?

Changed 7 years ago by damo

comment:2 Changed 7 years ago by damo

The link is deleted by selecting the link text and pressing the delete key.

I've repeated this on the nightly build. Attaching a screen shot to illustrate.

comment:3 Changed 7 years ago by garry.yao

  • Resolution set to wontfix
  • Status changed from new to closed

The link deletion logic with keystroke is browser implicit, you need to

  1. IE:
    • Press N+1 Backspace at the end of link text;
    • Press N Del at beginning of link text;
  2. FF:
    • Press N+1 Backspace at the end of link text;
    • Press N+1 Del at the beginning of link text;
  3. Safari: Press exactly N times with both Backspace and Del

Where N represent the text count of your link.
Sorry, we're not likely to fix the problem at this moment, hopefully above info is helpful to you.

comment:4 Changed 7 years ago by garry.yao

  • Keywords Confirmed added; WorksForMe removed

comment:5 Changed 7 years ago by fredck

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I also find this one quite strange.

The fact is that, during the editor usage, we could have some empty <a> being left in the DOM, due to the implicit browser behavior to some operations. This link deletion is a case. In V2, we simply cleanup empty <a> (with no "name" attribute), but it looks like this is not happening with V3. We could simply implement the same cleanup rule here.

Note that the attached screenshot precisely shows that there is also a <br> inside the link, and in that case it's correct to not have the link deleted because the <br> has not been selected before deletion.

In any case, I'm able to confirm this one, with IE only though, even without the <br>:

  1. Load the following HTML:
<p><a href="http://example.com">Test</a></p>
  1. Double click on "Test" to select it.
  1. Hit DEL to delete it.
  1. Switch back to source. You will have the following, which is wrong:
<p>
	<a href="http://example.com"></a></p>

comment:6 Changed 7 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from reopened to new

Changed 7 years ago by garry.yao

comment:7 follow-up: Changed 7 years ago by garry.yao

  • Keywords Review? added
  • Status changed from new to assigned
  1. The patch is including the latest patch at #3789 for easy reviewing;
  2. The empty paragraph removing logic is not respected after remove the empty link is out the scope of this ticket.

comment:8 Changed 7 years ago by fredck

  • Keywords Review+ added; Review? removed

#3789, so please attention when applying the patch.

comment:9 in reply to: ↑ 7 Changed 7 years ago by fredck

Replying to garry.yao:

  1. The empty paragraph removing logic is not respected after remove the empty link is out the scope of this ticket.

Please open a ticket for it (for the 3.1).

comment:10 Changed 7 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [3782].
The new ticket is opened as #3841 for the mentioned problem.

comment:11 follow-up: Changed 7 years ago by damo

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm not sure what part of this defect is fixed and what remains to be fixed still? I can reproduce this issue as described in the original problem description.

comment:12 Changed 7 years ago by garry.yao

The subject of this fix is removing the empty <a> element unless it represent an anchor.
WFM, could you provide more details?

comment:13 in reply to: ↑ 11 Changed 7 years ago by fredck

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to damo:

I'm not sure what part of this defect is fixed and what remains to be fixed still? I can reproduce this issue as described in the original problem description.

Please check the third paragraph in my previous comment ("Note that...").

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