More post slug cleaning

[honza.skypala]

Hi there,

removing a number of funky characters from post slugs in WP 3.3 is a great improvement, I suggest two more characters:

• „ (lower curly quotes, used by some european languages) - remove from slug
• × (multiplay sign) - substitute with letter x

Thx. Take care,

Honza

[SergeyBiryukov]

19820.patch​ adds the remaining ​quotation marks and &times sign to the replacement list.

[kurtpayne]

Fixes a unit test case

[kurtpayne]

Unit test for quote entities and multiply sign

[kurtpayne]

19820.2.patch​ fixes this test case:

$this->assertEquals("one-two", sanitize_title_with_dashes("One & Two;", '', 'save'));

Output was:

There was 1 failure:

1) TestSanitizeTitleWithDashes::test_strips_entities
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'one-two'
+'one'

This change is unrelated to SergeyBiryukov's patch, but is related to the request for unit tests.

SergeyBiryukov's patch looks good.

[SergeyBiryukov]

Replying to kurtpayne:

19820.2.patch​ fixes this test case:

$this->assertEquals("one-two", sanitize_title_with_dashes("One & Two;", '', 'save'));

This test case doesn't fail anymore, seems to be fixed elsewhere.

Committed unit tests: ​[UT688]

[nacin]

The test is marked as skipped against #10823.

php wp-test.php -p -f -t TestSanitizeTitleWithDashes
.......F......

Time: 1 second, Memory: 66.75Mb

There was 1 failure:

1) TestSanitizeTitleWithDashes::test_strips_entities
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'one-two'
+'one'

/Users/nacin/Sites/tests/wp-testcase/test_includes_formatting.php:1031
/Users/nacin/Sites/tests/wp-testlib/base.php:604
/Users/nacin/Sites/tests/wp-test.php:209

[nacin]

Is the change from . to \S intentional/necessary? We have found that \s (and therefore I imagine \S) incorrectly targets some letters in other languages or elsewhere in UTF8. A good example to test for is the Hebrew letter nun.

[SergeyBiryukov]

Replying to nacin:

The test is marked as skipped against #10823.

Thanks, didn't notice that. There are some alternative suggestions for the regex, we should probably leave this change for that ticket.

[kurtpayne]

Replying to nacin:

Is the change from . to \S intentional/necessary? We have found that \s (and therefore I imagine \S) incorrectly targets some letters in other languages or elsewhere in UTF8. A good example to test for is the Hebrew letter nun.

It was intentional at the time (I didn't know about #10823). A patch for ampersand handling belongs in #10823, though.

[kurtpayne]

Removed the patch that belongs in 10823

[kurtpayne]

For clarity: 19820.patch (SergeyBiryukov) and 19820.3.patch (my latest one, with the patch for 10823 removed) are the same thing. Please disregard core patches from me on this ticket.

[nacin]

In [20686]:

Add characters to be stripped or replaced in sanitize_title_with_dashes().

• Replace times (multiplication sign) with x.
• Strip low quotation marks and other curly quotes we don't already deal with.

props SergeyBiryukov. fixes #19820.