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

Allow embed <iframe> in job description content #1722

Merged
merged 4 commits into from
Apr 26, 2019

Conversation

jom
Copy link
Member

@jom jom commented Apr 24, 2019

Fixes issue of embeds getting removed from job description content. Introduced by #1495 in 1.31.1 (July 2018).

Fixes #1584
Fixes #1718

Changes proposed in this Pull Request:

  • Allows iframe in job description content.

Testing instructions:

  • Add YouTube link in job description.
  • Verify embed works correctly on frontend.

To Do

  • Add tests

@jom jom added this to the 1.33.0 milestone Apr 24, 2019
@jom jom requested review from alexsanford and donnapep April 24, 2019 10:46
@jom jom force-pushed the fix/job-description-escaping branch from 94862b0 to fb8d890 Compare April 24, 2019 10:54
Allows for YouTube embeds
@jom jom force-pushed the fix/job-description-escaping branch from fb8d890 to f0e62d8 Compare April 24, 2019 16:19
@tripflex
Copy link
Collaborator

While I understand the need for something like this --- doesn't allowing iframe in job content open up sites for potential exploitation by injecting malicious iframes?

@jom
Copy link
Member Author

jom commented Apr 24, 2019

@tripflex We could add wp_kses_post before it passes through the_job_description (which adds the embeds; like the_content filters). I’d be fine with that and should still fix this issue.

Trying it out here: 49c4927

Seems to still work. It adds a bit of extra overhead, but probably worth it.

@jom jom changed the title Allow <iframe> in job description content Allow embed <iframe> in job description content Apr 24, 2019
Copy link
Contributor

@alexsanford alexsanford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work well. One question about how the kses is happening, but otherwise I think this one looks good!

wp-job-manager-template.php Show resolved Hide resolved
@jom jom merged commit f2d25f6 into master Apr 26, 2019
@jom jom deleted the fix/job-description-escaping branch April 26, 2019 07:39
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.

Youtube video embeds not working Allow additional HTML entities in job listing post content
3 participants