Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unlinking fails for middle block when more than 2 blocks are selected #8030

Closed
oleq opened this issue Sep 4, 2020 · 3 comments · Fixed by #8495
Closed

Unlinking fails for middle block when more than 2 blocks are selected #8030

oleq opened this issue Sep 4, 2020 · 3 comments · Fixed by #8495
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine package:link squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@oleq
Copy link
Member

oleq commented Sep 4, 2020

📝 Provide detailed reproduction steps (if any)

  1. Create 3 blocks, e.g. used 3 list items.
  2. Select all of them and create a link.
  3. Click "Unlink" in the link actions balloon.

✔️ Expected result

All items should be free of the link.

❌ Actual result

The middle item is still linked.

I briefly checked this and the 3 ranges Writer#removeAttribute() works on internally are looking fine. It feels like the bug is in the setAttributeOnRange() helper. When I logged the following

console.log( val.item, valueBefore, valueAfter );

in the walker inside it was

TextProxy {textNode: Text, data: "tem", offsetInText: 0} undefined "foo"
Element {parent: RootElement, _attrs: Map(2), name: "listItem", _children: NodeList} undefined undefined
TextProxy {textNode: Text, data: "third", offsetInText: 0} undefined "foo"

because the walker created with shallow: true. When I got rid of this option, the bug is gone and more items are logged

TextProxy {textNode: Text, data: "tem", offsetInText: 0} undefined "foo"
Element {parent: RootElement, _attrs: Map(2), name: "listItem", _children: NodeList} undefined undefined
TextProxy {textNode: Text, data: "second item", offsetInText: 0} undefined "foo"
Element {parent: RootElement, _attrs: Map(2), name: "listItem", _children: NodeList} "foo" undefined
TextProxy {textNode: Text, data: "thir", offsetInText: 0} undefined "foo"

P.S. The bug could also be in getMinimalFlatRanges().

📃 Other details

  • Browser: Chrome
  • OS: Mac
  • CKEditor version: v22.0.0

It's not a regression, I checked in v16.0.0 and it's all the same.


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@oleq oleq added type:bug This issue reports a buggy (incorrect) behavior. package:engine package:link domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. labels Sep 4, 2020
@panr
Copy link
Contributor

panr commented Sep 11, 2020

This bug appeared after the changes from this commit: 0ed523e#diff-a3ae70b600cbe7a13e75771a6a5519acR81

I would label it as type:regression. However, I know some of those changes were intentional...

@oleq
Copy link
Member Author

oleq commented Sep 11, 2020

Are you sure this is the right commit? Because I'm experiencing the same bug in releases as old as v16.0.0.

@panr
Copy link
Contributor

panr commented Sep 11, 2020

git bisect told me that this is the one ;-P Reverting that commit fixes this issue for my case (regarding multi-block link marker issue).

@Mgsy Mgsy added this to the next milestone Sep 14, 2020
@pomek pomek modified the milestones: next, nice-to-have Nov 10, 2020
@niegowski niegowski modified the milestones: nice-to-have, iteration 38 Nov 20, 2020
@niegowski niegowski self-assigned this Nov 20, 2020
Reinmar added a commit that referenced this issue Nov 23, 2020
Fix (link): Fixed the `unlink` command for the selection spreading over 3+ blocks. Closes #8030.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine package:link squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants