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

Configuration flag forceSimpleAmpersand does not work #965

Closed
alexmaris opened this issue Sep 26, 2017 · 7 comments
Closed

Configuration flag forceSimpleAmpersand does not work #965

alexmaris opened this issue Sep 26, 2017 · 7 comments
Labels
good first issue Relatively easy to fix. This is a perfect issue if you are willing to create a Pull Request. regression This issue is a regression. 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

@alexmaris
Copy link
Contributor

Are you reporting a feature request or a bug?

Bug

Check if the issue is already reported

https://dev.ckeditor.com/ticket/16746#no1
Previously reported in this ticket, but the steps to replicate were not correct. The documentation states that the forceSimpleAmpersand flag will "force using & instead of & in element attributes values."

(https://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-forceSimpleAmpersand)

Provide detailed reproduction steps (if any)

  1. Create a new editor with the forceSimpleAmpersand option set to true (https://codepen.io/anon/pen/gGmOmW?editors=1010)
  2. Switch to Source View
  3. Add an <a> tag with an href attribute that contains an ampersand ex: <a href="http://www.blah.com?foo=1&bar=2">Test link</a>
  4. Switch WYSIWYG View
  5. Switch back to Source View

Expected result

Anchor tag href property should remain the same:
<a href="http://www.blah.com?foo=1&bar=2">Test link</a>

Actual result

Anchor tag href property has the ampersand encoded
<a href="http://www.blah.com?foo=1&amp;bar=2">Test link</a>

Other details

  • Browser: Chrome Version 60.0.3112.113 (Official Build) (64-bit)
  • OS: MacOS
  • CKEditor version: 4.7.1
  • Installed CKEditor plugins: default plugins
alexmaris added a commit to alexmaris/ckeditor-dev that referenced this issue Sep 26, 2017
Run ampersand replacement after the  htmlEncodeAttr, otherwise the results are overwritten (ckeditor#965)
@beatadelura beatadelura self-assigned this Sep 27, 2017
@beatadelura beatadelura added status:confirmed An issue confirmed by the development team. type:bug A bug. labels Sep 27, 2017
@beatadelura beatadelura removed their assignment Sep 27, 2017
@mlandmann
Copy link

mlandmann commented Nov 28, 2017

The sad facts are some websites do not support the & amp; and expect only the & to be there. thus as is -- CKeditor breaks the URL. Who gives a damn about the specs -- CKEditor needs to fix this and let the users control what is needed and not force something that doesn't work (sometimes)

@j-koehler
Copy link

Looks like this problem was introduced with 4.5.0 - with 4.4.8 it works as expected.

@siemas
Copy link

siemas commented Sep 10, 2018

@alexmaris your commit solves the problem (bug) for me. Cloud you make a pull request for this fix?

@alexmaris
Copy link
Contributor Author

Ahh yes I can, I think I was in the middle of writing a unit test for it but was pulled away. I'll submit a PR in a few.

alexmaris added a commit to alexmaris/ckeditor-dev that referenced this issue Sep 14, 2018
Run ampersand replacement after the  htmlEncodeAttr, otherwise the results are overwritten (ckeditor#965)
@alexmaris
Copy link
Contributor Author

PR submitted, sorry for the delay!

@mlewand mlewand added regression This issue is a regression. 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 Sep 14, 2018
mlewand pushed a commit that referenced this issue Sep 14, 2018
Run ampersand replacement after the  htmlEncodeAttr, otherwise the results are overwritten (#965)
@Comandeer Comandeer added this to the 4.10.2 milestone Sep 14, 2018
@mlewand mlewand modified the milestones: 4.10.2, 4.11.0 Oct 4, 2018
@mlewand mlewand changed the title Bug: configuration flag forceSimpleAmpersand does not work Configuration flag forceSimpleAmpersand does not work Nov 5, 2018
@mlandmann
Copy link

The sad facts are some websites do not support the & amp; and expect only the & to be there. thus as is -- CKeditor breaks the URL. Who gives a damn about the specs -- CKEditor needs to fix this and let the users control what is needed and not force something that doesn't work (sometimes)

I wrote this several years ago, and today 6/24/20 it is still an issue. forceSimpleAmpersand does not work. CKeditor people,,you are not Google. We the users of your product wear big pants and can deal with backward compatibility. Will you get this fixed and stop hiding behind some outdated standards. I just spent hours on this today. I have better things to do with my life.

@f1ames
Copy link
Contributor

f1ames commented Jun 25, 2020

@mlandmann I see this issue was fixed some time ago in #2416 and #2418. However, if you still experiencing any problems with this configuration option please open a new issue describing your case and reproduction steps.

drupal-ckeditor-libraries-group-machine pushed a commit to drupal-ckeditor-libraries-group/htmlwriter that referenced this issue Dec 2, 2021
Run ampersand replacement after the  htmlEncodeAttr, otherwise the results are overwritten (ckeditor/ckeditor4#965)
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. regression This issue is a regression. 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

No branches or pull requests

8 participants