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

CKEditorConfig - Fix double-escaped slashes #11747

Merged
merged 1 commit into from
Mar 2, 2018

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Mar 2, 2018

Overview

The "Advanced Options" section of the CKEditor configurator allows user-input strings. Slashes were being escaped multiple times in that input. This fixes it.

To reproduce the bug:

  • Add an advanced option and enter a string containing slashes.
  • Click "Save"
  • Observe on reloading the form the string has changed, adding extra escaping slashes that should not be there.

Before

screenshot from 2018-03-01 20 55 46

After

screenshot from 2018-03-01 20 55 16

@seamuslee001
Copy link
Contributor

@colemanw is there a ticket for this? what is the issue here?

@colemanw
Copy link
Member Author

colemanw commented Mar 2, 2018

@seamuslee001 you were too fast for me. I was just writing the description.

@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.1)

  • (r-explain) Pass Explained well enough on the PR what is happening here
  • (r-test) Pass no tests needed
  • (r-code) Pass
  • (r-doc) Pass
  • (r-maint) Pass
  • (r-run) Pass Observed the previous behavior and after applying patch observed URL is correct and this is safe
  • (r-user) Pass
  • (r-tech) Pass

@seamuslee001
Copy link
Contributor

Discussed this with Tim and we feel there isn't any potential issues that might be generated from the non escaping of / Merging

@seamuslee001 seamuslee001 merged commit ebc2ffb into civicrm:master Mar 2, 2018
@colemanw colemanw deleted the CKEditorConfig branch March 2, 2018 14:41
@colemanw
Copy link
Member Author

colemanw commented Mar 2, 2018

Thanks @seamuslee001. I also recommend up-voting this excellent post which identified this bug and demonstrated some nice advanced features: https://civicrm.stackexchange.com/a/22957/4

@mlutfy mlutfy added this to the 4.7.32 milestone Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants