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

[AdminBundle] Fix escaping of img src attributes in WYSIWYG fields #1858

Merged
merged 1 commit into from Mar 5, 2018

Conversation

mtnorthrop
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets #1857

The MediaTokenTransformer::reverseTransform() method uses the Crawler::html() method to produce the HTML it returns, but this method automatically URL-encodes the src attribute of image tags.

This PR does a search and replace on the final HTML to make sure that the square brackets used for the tokens are preserved.

NB: the Symfony docs mention that the DomCrawler Component is not designed to re-dump HTML, and so it might be worth considering looking for a different tool to use in this class. (See https://symfony.com/doc/current/components/dom_crawler.html#component-dom-crawler-dumping).

@wesleylancel
Copy link
Contributor

wesleylancel commented Mar 1, 2018 via email

@mtnorthrop
Copy link
Contributor Author

No problem, I've updated the PR to decode <a> tags as well. I'm doing the search and replace only within specific tags, and not on everything in the form field, in order to avoid reintroducing the bug that was reported in #1826.

@wesleylancel
Copy link
Contributor

@mtnorthrop Thanks!

This is quite an annoying bug, any chance of a quick merge and release?

@mtnorthrop
Copy link
Contributor Author

mtnorthrop commented Mar 3, 2018

I hope this can be reviewed, merged and released soon, because in AdminBundle 5.0.4 all internal links and images added to the WYSIWYG fields are currently broken, and this PR fixes the issue.

@sandergo90 sandergo90 merged commit a6e7c2d into Kunstmaan:5.0 Mar 5, 2018
@sandergo90
Copy link
Contributor

@mtnorthrop I've reviewed it and merged it now. We will create a new release tomorrow!

@mtnorthrop
Copy link
Contributor Author

@sandergo90 Thank you!

@mtnorthrop mtnorthrop deleted the issue_1857 branch March 5, 2018 08:45
@mtnorthrop mtnorthrop restored the issue_1857 branch March 5, 2018 08:50
@mtnorthrop mtnorthrop deleted the issue_1857 branch March 5, 2018 08:50
@sandergo90 sandergo90 added this to the 5.0.5 milestone Mar 5, 2018
@sandergo90
Copy link
Contributor

@mtnorthrop new release submitted!

Ch3F24 pushed a commit to Ch3F24/KunstmaanBundlesCMS that referenced this pull request Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants