-
Notifications
You must be signed in to change notification settings - Fork 368
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
Allows for the option (filter) for shortcodes to be stripped #1643
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works well as-is, but I wonder if we might want to take a different approach.
Are there any legitimate use-cases to accept shortcodes in the content being submitted from the frontend? I can understand wanting to add shortcodes from WP Admin, potentially, but I'm not sure it makes sense for frontend users to be able to submit shortcodes in their content.
Would something like this make more sense:
- We do not filter
the_content
to strip shortcodes (allowing the use of shortcodes from WP Admin). - When a listing is submitted from the frontend, we strip shortcodes before save (alternatively, is there a way to escape them so they display verbatim?)
- We could then have a filter that disables stripping or escaping shortcodes on submit, so site owners can opt into allowing frontend users to submit shortcodes if they want to.
Thoughts?
Also, this needs a rebase.
I based part of my conservative approach on @spencerfinnell's comment. Spencer, do you think we could escape shortcodes in frontend submissions? Most people definitely shouldn't be able to post shortcodes on the frontend. I guess the only situation that might pose a problem is if the site admin has edited a job to put in a shortcode and then it gets edited from the frontend (same as our block editor issue). @alexsanford Were you thinking the shortcode syntax would be escaped by default or when filtered on? |
Interesting...sounds like it's common to use shortcodes in the description, but I'm interested in whether it is common for users submitting/editing from the frontend. I had assumed not, because a frontend user, in general, wouldn't have any knowledge of WordPress. But based on @spencerfinnell's comments linked above I'm thinking I was wrong about that (Spencer, can you confirm that these submissions/edits you're describing are indeed from the frontend?) In any case, @jom you're right about the frontend vs. backend editing issue. Personally, I would still be in favour of stripping or escaping shortcodes in frontend submissions by default, but if there are significant backwards compatibility concerns, then I'm not sure it's worth the trouble. In which case, I like this solution. Having a simple true/false filter is nice for something like this, I think. |
I agree with @alexsanford the only other thing I could think of, would be to add some code to allow the admin users to specify "allowed" shortcodes that can be used, that way it's not just a global allow/deny for shortcodes in content area. I commonly provide snippets for users that allows them to execute shortcodes on specific fields when they ask about it. I also believe that it would be an issue as well if admin added shortcode from admin area, and then was edited from frontend, this would essentially strip out that shortcode the admin added. @spencerfinnell what are your thoughts on this? |
I've definitely seen sites where the submission page includes content telling users about shortcodes they can add. Not a huge use-case, but it exists. Totally happy with this solution since nothing changes automatically. |
@alexsanford Bringing back up this minor issue for consideration in 1.36.0. |
Added a snippet for this to the docs (https://wpjobmanager.com/document/wpjm-core-snippets/) |
Fixes #1146
I chose to strip the shortcodes earlier and check the post type instead of removing
do_shortcode
. Hopefully this would handle the case where the theme callsget_content
outside of a job manager template.Note: I'm not sure this should actually be merged. This still requires a snippet but could be completely replaced with another snippet added to our snippet library:
Changes proposed in this Pull Request:
job_manager_strip_job_shortcodes
returns true.Testing instructions:
add_filter( 'job_manager_strip_job_shortcodes', '__return_true' );