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
Fix issue with double escaped autolink special characters #4910
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, however there is one edge case that fails after the changes:
https://www.google.com/test/?one=one&=1
If link contains intentional &
sequence, it's converted to &
instead of &
. The same happens with <
and other entities mentioned in allEscRegex
in tools.js
file.
The problem with the issue you mentioned is that there is no way (at the moment) to differentiate both escaped and unescaped characters when manipulating HTML in
Both clipboard and typing operate on already processed HTML by a browser using copybin or text matcher. This issue also exists in the older |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I've got just some doubts about unit test (see my inline comment).
Co-authored-by: Tomasz Jakut <vepomoc@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
What is the purpose of this pull request?
Bug fix
Does your PR contain necessary tests?
All patches that 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
Did you follow the CKEditor 4 code style guide?
Your code should follow the guidelines from the CKEditor 4 code style guide which helps keep the entire codebase consistent.
What is the proposed changelog entry for this pull request?
What changes did you make?
Links were double escaped by executing 2x
setAttribute
method with already escaped&
character to&
.Which issues does your PR resolve?
Closes #4858