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

Special entities in entities_additional may break escaping #2448

Closed
Deroin opened this issue Oct 1, 2018 · 2 comments · Fixed by #2487
Closed

Special entities in entities_additional may break escaping #2448

Deroin opened this issue Oct 1, 2018 · 2 comments · Fixed by #2487
Assignees
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Milestone

Comments

@Deroin
Copy link

Deroin commented Oct 1, 2018

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Initialize CKE with entities_additional set to 'apos'
  2. Enter text amp' into the editor
  3. Switch to source view

Expected result

Source view shows <p>amp&apos;</p>

Actual result

Source view shows <p>&undefined;&undefined;&undefined;&apos;</p>

Other details

  • Browser: Firefox, Chrome
  • OS: Debian, Windows
  • CKEditor version: 4.10.1
  • Installed CKEditor plugins: Default plugins as in development version

I pinned this down to be caused by plugins/entities/plugin.js::buildTable. During the replacement of entities in line 63 any special entity will be removed from the entities list together with the following ,. If there is no comma (because it is the last value) the resulting ends with a comma.

This causes entities.split(...) to create an array with empty string as last value while the join then creates a string ending with &; which will be replaced by &amp;; when reading inner HTML.
Due to this a, m and p will be interpreted as entities by CKE.

@msamsel msamsel self-assigned this Oct 2, 2018
@msamsel msamsel added type:bug A bug. status:confirmed An issue confirmed by the development team. labels Oct 2, 2018
@msamsel
Copy link
Contributor

msamsel commented Oct 2, 2018

I was able to reproduce the issue here: https://codepen.io/msamsel/pen/GYpEvv

@msamsel msamsel removed their assignment Oct 2, 2018
@mlewand mlewand closed this as completed Oct 16, 2018
@mlewand mlewand reopened this Oct 16, 2018
@mlewand
Copy link
Contributor

mlewand commented Oct 16, 2018

Closed wrong issue, reopening.

@jacekbogdanski jacekbogdanski self-assigned this Oct 16, 2018
@mlewand mlewand added target:minor Any docs related issue that can be merged into a master or major branch. good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. labels Oct 24, 2018
@mlewand mlewand added this to the 4.11.0 milestone Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. status:confirmed An issue confirmed by the development team. target:minor Any docs related issue that can be merged into a master or major branch. type:bug A bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants