Opened 4 years ago

Closed 20 months ago

Last modified 18 months ago

#13062 closed Bug (fixed)

Can't unlink links

Reported by: agunescu Owned by: Tomasz Jakut
Priority: Nice to have (we want to work on it) Milestone: CKEditor 4.7.0
Component: General Version: 4.0 Beta
Keywords: Cc:

Description

When the cursor is positioned before or after the text, the unlink button is active but if you click it nothing happens.

Once I added a link, I can't unlink it. "Unlink" button is not working.

Change History (25)

comment:1 Changed 4 years ago by Jakub Ś

Resolution: invalid
Status: newclosed

There is one general condition for unlinking - selection has to start within link.

To unlink whole link you have to select it. You can use element's path for that (once cursor is inside the link) or select whole link with mouse.

You can unlink part of the link e.g select half of the link and lot's of text behind it (it doesn't matter). Part of selected link will be unlinked. This is how it works.


When the cursor is positioned before or after the text, the unlink button is active

This is not happening in default editor. Behaviour is as written above. Perhaps you have custom plugins installed that cause this or made some core code modifications or use some browser plugins that influence this behaviour.

I’m closing this issue as invalid but if you are able to reproduce it on demo page or in default CKEditor, please provide exact steps to do so.

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

To unlink whole link you have to select it. You can use element's path for that (once cursor is inside the link) or select whole link with mouse.

You can also use context menu's option "Unlink".

When the cursor is positioned before or after the text, the unlink button is active

This is not happening in default editor. Behaviour is as written above. Perhaps you have custom plugins installed that cause this or made some core code modifications or use some browser plugins that influence this behaviour.

I'm able to reproduce this on Chrome. You need to carefully place the selection at the end of a link so the elements path shows that it's inside <a> (navigate using the right arrow key). Then the unlink button does nothing except moving the selection outside of that link. This is a bug, because wherever else in a link the unlink button removes that link completely.

comment:3 Changed 4 years ago by Jakub Ś

Resolution: invalid
Status: closedreopened

Thank for pointing that out however I would not figure it out from original ticket report :)

comment:4 Changed 4 years ago by Jakub Ś

Status: reopenedconfirmed
Version: 4.0 Beta

Sorry for the confusion - the problem is that I use unlink option with link selected manually or selected with element's path. With time I have forgotten about "single click inside link->unlink" option.

Now I can confirm the ticket which can be reproduced from CKEditor 4.0 beta (works fine in 3.6.x) in every browser

To reproduce:

  1. Load replacebycode sample
  2. Click at the beginning or at the end of link
  3. Make sure elements path is showing that you are inside 'a' tag and unlink button is active
  4. Click unlink - button gets deactivated but link doesn't get unlinked (Element's path shows that you are not inside the link anymore).

comment:5 Changed 3 years ago by agunescu

Hi,

Can I ask what is the status of this ticket? Is there any chance to get the unminified code so I can fix it on my own?

Thanks

comment:6 Changed 3 years ago by Jakub Ś

No one is working on this ticket at the moment.

If you want to resolve this ticket on your own, you are welcome to get source code according to http://docs.ckeditor.com/#!/guide/dev_source. You are also welcome to submit a a pull request if you want. Please however get familiar with http://docs.ckeditor.com/#!/guide/dev_contributing and https://help.github.com/articles/creating-a-pull-request before submitting it.

comment:7 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5

comment:8 Changed 3 years ago by Tomasz Jakut

Owner: set to Tomasz Jakut
Status: confirmedassigned

comment:9 Changed 3 years ago by Tomasz Jakut

Status: assignedreview

The problem was in CKEDITOR.style, in the way of handling boundary elements - they weren't removed even if alwaysRemoveElement was true. Pushed fix to branch:t/13062.

comment:10 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5CKEditor 4.5.6

comment:11 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:12 Changed 3 years ago by Marek Lewandowski

Status: reviewreview_failed

I've rebased your solution to the latest master.

[https://github.com/cksource/ckeditor-dev/blob/ab592b9 /core/style.js#L1080 This line] will cause style in whole block to be removed.

Example TC:

  1. Open any sample with CKEditor. (i.e. samples/index.html)
  2. Using Source button set following source markup:
<p>I&#39;m<a href="http://foo"> an </a>in<a href="http://bar">sta</a>nce of <a href="http://ckeditor.com">CKEditor</a>.</p>
  1. Put the selection in a following place: I'm an instance of CKEditor^.
  2. Press unlink button.

Expected result:
Only link next to collapsed selection should be affected.

Current result:
Every link in block is removed.

Last edited 3 years ago by Marek Lewandowski (previous) (diff)

comment:13 Changed 3 years ago by Tomasz Jakut

Status: review_failedreview

I modified my solution and updated tests. There was also some nasty bug in IE/Edge, which moved selection outside of the link. I didn't find the source of it, but I prepared small workaround.

Pushed changes to branch:t/13062.

comment:14 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:15 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:16 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:17 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:18 Changed 2 years ago by Tade0

Status: reviewreview_failed

The main focus of this ticket is completed, but there's a problem with that workaround: In the manual test in Edge it's impossible to have both the link in the elements path and the selection collapsed at the beginning or end of the first paragraph only.

In other words: the workaround does not work for the first paragraph in the manual test.

EDIT: rebased with current master and pushed to branch:t/13062.

Last edited 2 years ago by Tade0 (previous) (diff)

comment:19 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.6.1

comment:20 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1CKEditor 4.6.2

Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.

comment:21 Changed 22 months ago by Marek Lewandowski

Milestone: CKEditor 4.6.2
Priority: NormalNice to have (we want to work on it)

*t.jakut* won't be able to work on it in upcoming weeks. Let's move it into the backlog.

comment:22 Changed 21 months ago by Tomasz Jakut

Status: review_failedreview

Works for me, however I must admit that putting selection inside link in the ending of paragraph inside Edge is quite troublesome (this behavior is visible also for other styles, e.g. bold). But when I managed to do it, I could unlink the link.

I've rebased the fix to the latest major and updated tags in tests. Pushed branch:t/13062.

comment:23 Changed 21 months ago by Tomasz Jakut

I've updated unit test, because in the current form it was failing on the CI for built version of CKE4. I've also renamed manual test to something more human friendly.

comment:24 Changed 20 months ago by Tade0

Resolution: fixed
Status: reviewclosed

I was wondering how you managed to not trigger the line-length JSCS check, but apparently tabs are treated by this system as one character.

Overall LGTM.

Fixed with git:7ee2b8ab657294c35832ceb1e0bb823bb7df9940

comment:25 Changed 18 months ago by Marek Lewandowski

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