Opened 5 years ago
Last modified 3 years ago
#47594 accepted enhancement
Add nbsp symbol codes to convert throught sanitize_title
Reported by: | hokku | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | minor | Version: | 5.3 |
Component: | Formatting | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
Default filters needs to be supplemented nbsp codes such as
[ '%e2%80%af', '%e2%80%87', '%e2%81%a0' ]
and
[ ' ', ' ', ' ', ' ', '⁠', '⁠' ]
Attachments (3)
Change History (33)
#2
@
5 years ago
- Summary changed from Add nbsp symbol codes to convert to sanitize_title to Add nbsp symbol codes to convert throught sanitize_title
#3
@
5 years ago
- Milestone changed from Awaiting Review to 5.3
- Owner set to SergeyBiryukov
- Status changed from new to accepted
#6
@
5 years ago
- Keywords reporter-feedback 2nd-opinion added
I need to clarify something so I don't get it wrong.
I'm missing some context here too :)
@SergeyBiryukov are you looking at this for 5.3? I'm not sure I understand what's being fixed and why.
#7
@
5 years ago
- Milestone changed from 5.3 to Future Release
With version 5.3 Beta 1 releasing today, I'm moving this to Future Release
.
This ticket was mentioned in Slack in #core by metalandcoffee. View the logs.
4 years ago
#14
@
4 years ago
Per Sergey during today's scrub:
Will try to address before beta, or punt otherwise.
#15
@
4 years ago
- Keywords needs-refresh added
Marking for a refresh as the patch no longer applies cleanly to trunk
.
#16
@
4 years ago
- Milestone changed from 5.6 to 5.7
Didn't get a chance to write the tests for this in time for 5.6 Beta 1, moving to the next release for now.
This ticket was mentioned in Slack in #core by lukecarbis. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#20
@
4 years ago
- Keywords 2nd-opinion needs-refresh removed
@hellofromTonya @SergeyBiryukov I refreshed and simplified the previous patch as per today's bug scrub.
#21
@
4 years ago
Thank you @audrasjb! I'll get the unit tests written for it. And it should be ready for commit
consideration.
This ticket was mentioned in PR #915 on WordPress/wordpress-develop by hellofromtonya.
4 years ago
#22
- Keywords has-unit-tests added; needs-unit-tests removed
Trac ticket: https://core.trac.wordpress.org/ticket/47594
- Applies patch 47594.1.diff. Props @Adhitya03 and @audrasjb.
- Adds unit tests
- Also renames test file to comply with handbook test standard and adds
@covers
hellofromtonya commented on PR #915:
4 years ago
#23
Before the patch, all tests fail except for
:
.FFFFFFFFF 10 / 10 (100%) Time: 6.18 seconds, Memory: 116.00 MB There were 9 failures: 1) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #1 ('%e2%80%af') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handle%e2%80%afspace%e2%80%afcharacter' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 2) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #2 ('%e2%80%87') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handle%e2%80%87space%e2%80%87character' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 3) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #3 ('%e2%81%a0') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handle%e2%81%a0space%e2%81%a0character' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 4) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #4 (' ') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handlespacecharacter' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 5) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #5 (' ') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handlespacecharacter' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 6) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #6 (' ') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handlespacecharacter' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 7) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #7 (' ') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handlespacecharacter' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 8) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #8 ('⁠') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handlespacecharacter' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26 9) Tests_Formatting_SanitizeTitle::test_unicode_space_characters with data set #9 ('⁠') Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -'handle-space-character' +'handlespacecharacter' /var/www/tests/phpunit/tests/formatting/sanitizeTitle.php:26
#24
@
4 years ago
- Keywords commit added
Applied patch. Works as expected in the unit tests and in manual live testing. Marking this ticket as a commit
candidate. @SergeyBiryukov it's ready for your review.
#25
@
4 years ago
- Keywords commit removed
Why are the changes part of sanitize_title()
and not sanitize_title_with_dashes()
which already handles some of the cases?
The list also seems to be incomplete. I think if we're going to update the list we should cover all space characters. Here's a filter I have recently used (which is also incomplete):
<?php /** * Extends sanitize_title_with_dashes() to replace more space related characters. * * @param string $title Sanitized title. * @param string $raw_title The title prior to sanitization. * @param string $context The context for which the title is being sanitized. * @return string The sanitized string. */ function trac_replace_spaces_in_titles( string $title, string $raw_title, string $context ): string { if ( 'save' !== $context ) { return $title; } $title = str_replace( [ '%e2%80%89', // THIN SPACE. ' ', // THIN SPACE. ' ', // THIN SPACE. '%e2%80%8a', // HAIR SPACE. '%e2%80%91', // NON-BREAKING HYPHEN. '%e2%80%92', // FIGURE DASH. '%e2%80%af', // NARROW NO-BREAK SPACE. ' ', // NARROW NO-BREAK SPACE. ], '-', $title ); $title = preg_replace( '|-+|', '-', $title ); $title = trim( $title, '-' ); return $title; } add_filter( 'sanitize_title', 'trac_replace_spaces_in_titles', 10, 3 );
#26
@
4 years ago
- Milestone changed from 5.7 to 5.8
5.7 Beta 1 is happening in shortly. Ran out of time to get this ticket into the release. Punting to 5.8.
If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.
Hi,I wanna make a patch for this but I need clarify something so I don't get it wrong.
In this case, do mean "Default filters needs to be supplemented nbsp" is to sanitize all kind of Unicode characters of SPACE to be its HTML Entity " " ?