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

Allows for the option (filter) for shortcodes to be stripped #1643

Closed
wants to merge 3 commits into from

Conversation

jom
Copy link
Member

@jom jom commented Dec 11, 2018

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 calls get_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:

add_filter( 'the_content', function( $content ) {
	if ( 'job_listing' !== get_post_type() ) {
		return $content;
	}
	return strip_shortcodes( $content );
} );

Changes proposed in this Pull Request:

  • Adds option to strip shortcodes from content if job_manager_strip_job_shortcodes returns true.

Testing instructions:

  • Add job listing with a shortcode.
  • Verify the shortcode appears when viewing it on the frontend.
  • Add snippet:
    add_filter( 'job_manager_strip_job_shortcodes', '__return_true' );
  • Refresh job listing and verify the shortcode doesn't appear.

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.

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.

@jom
Copy link
Member Author

jom commented Dec 11, 2018

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?

@alexsanford
Copy link
Contributor

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.

@tripflex
Copy link
Collaborator

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?

@spencerfinnell
Copy link
Contributor

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.

@jom jom modified the milestones: 1.32.0, Next major Dec 17, 2018
@jom jom requested review from a team and removed request for spencerfinnell January 27, 2021 12:20
@jom jom modified the milestones: Next major, 1.36.0 Jan 27, 2021
@jom jom requested review from alexsanford and removed request for a team January 27, 2021 12:24
@jom
Copy link
Member Author

jom commented Jan 27, 2021

@alexsanford Bringing back up this minor issue for consideration in 1.36.0.

@alexsanford alexsanford removed their request for review June 10, 2022 19:46
@donnapep donnapep removed their request for review January 13, 2023 14:23
@yscik yscik removed this from the 1.36.0 milestone Jun 27, 2023
@yscik yscik removed the request for review from tripflex August 7, 2023 14:32
@yscik
Copy link
Contributor

yscik commented Aug 7, 2023

Added a snippet for this to the docs (https://wpjobmanager.com/document/wpjm-core-snippets/)

@yscik yscik closed this Aug 7, 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.

Disable shortcode use in job submission
5 participants