Make WordPress Core

Opened 5 years ago

Last modified 5 years ago

#47849 reviewing defect (bug)

Post excerpts accept spaces

Reported by: adeltahri's profile adeltahri Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 5.2.2
Component: Posts, Post Types Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Post excerpts accept spaces.
This is image shows the spaces https://d.pr/i/2th5zq

Attachments (4)

Screen Shot on 2019-08-08 at 00-00-33.png (7.9 KB) - added by adeltahri 5 years ago.
47849.diff (2.4 KB) - added by donmhico 5 years ago.
Prevent saving of only whitespaces post excerpt + unit test.
47849.2.diff (2.4 KB) - added by donmhico 5 years ago.
Corrected @since to adhere WPCS.
47849.3.diff (2.3 KB) - added by donmhico 5 years ago.

Download all attachments as: .zip

Change History (17)

#1 @SergeyBiryukov
5 years ago

  • Component changed from General to Posts, Post Types
  • Keywords reporter-feedback added

Hi @adeltahri, welcome to WordPress Trac! Thanks for the report.

The excerpt is a freeform text input, spaces seem like valid input to me. Could you clarify why it's an issue?

#2 @adeltahri
5 years ago

Hi @SergeyBiryukov I think if the excerpt contains only spaces it must be skipped and not saved in the database.
if the developer tested $post->post_excerpt if it's empty or not, it will return true even with spaces only so we need to use trim() on the test, I think it will be good if we validate the input like when we add a new comment https://d.pr/i/OrPjEi

#3 @SergeyBiryukov
5 years ago

  • Keywords needs-patch needs-unit-tests added; reporter-feedback removed
  • Milestone changed from Awaiting Review to Future Release

Makes sense, thanks for clarifying!

Unlike an empty comment, I don't think we can display an error for an empty excerpt, as that just means the excerpt will be dynamically generated by wp_trim_excerpt().

We can, however, use trim() before saving it in the database, so that an excerpt that only contains whitespace is saved as empty.

@donmhico
5 years ago

Prevent saving of only whitespaces post excerpt + unit test.

#4 @donmhico
5 years ago

  • Keywords has-patch added; needs-patch needs-unit-tests removed

#5 @SergeyBiryukov
5 years ago

  • Milestone changed from Future Release to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#6 @adeltahri
5 years ago

Thank you @donmhico for the fix, I test it and it works as expected.

#7 @mukesh27
5 years ago

  • Keywords needs-refresh added

@donmhico small change needed in the attached patch.

As per the documentation standards, let's change @since 5.3 to @since 5.3.0 in 47849.diff

@donmhico
5 years ago

Corrected @since to adhere WPCS.

#8 @donmhico
5 years ago

  • Keywords needs-refresh removed

@mukesh27 done. Thanks for pointing this out.

#9 @birgire
5 years ago

I recall that the combination empty( trim() ) did result in a fatal error in some older PHP versions.

To be sure I checked here:

https://3v4l.org/MKi5E

It runs successfully for PHP 5.5+, and since the minimum of WordPress is now at PHP 5.6 it should be ok.

For PHP 5.4- versions the error is:

Fatal error: Can't use function return value in write context in /in/MKi5E on line 3

ps. the tests in 47849.2.diff might need a slight WPCS adjuments, like the array alignments.

@donmhico
5 years ago

#10 @donmhico
5 years ago

  • Keywords has-unit-tests added

Thanks for pointing out @birgire. I've attached a new patch that aligns the array elements.

#11 @davidbaumwald
5 years ago

@SergeyBiryukov Is this on your list to merge for 5.3? The most recent patch still applies cleanly.

This ticket was mentioned in Slack in #core by marybaum. View the logs.


5 years ago

#13 @davidbaumwald
5 years ago

  • Milestone changed from 5.3 to Future Release

With 5.3 RC1 releasing today, this is being moved to Future Release. If any committer feels this can be worked in quickly for 5.3 or can assume ownership in 5.4, feel free to move it back up.

Note: See TracTickets for help on using tickets.