Make WordPress Core

Opened 11 months ago

Last modified 2 months ago

#64284 new defect (bug)

Convert times/multiplication symbol (×) into the letter "x" when sanitizing titles for slugs

Reported by: archmaster7 Owned by:
Priority: normal Milestone: Future Release
Component: Formatting Version:
Severity: minor Keywords: changes-requested has-patch has-unit-tests dev-feedback close early
Cc: Focuses:

Description (last modified by westonruter)

We had an issue where user thought they were entering an x (Latin Small Letter X (U+0078)) but they were actually entering the multiplication symbol (Multiplication Sign (U+00D7)). An "x" did not show up in the slug which lead to some issues. Might suggest disallowing Multiplication Sign (U+00D7) from titles.

The multiplication symbol can be replaced with "x" automatically when sanitizing the title for use in a slug. Core does similarly for dashes which get replaced with hyphens via sanitize_title_with_dashes(). This was touched in the 6.9 release in #64089.

Attachments (1)

64284.diff​ (1.6 KB ) - added by iflairwebtechnologies 11 months ago.

Download all attachments as: .zip

Change History (24)

#1 follow-up: @westonruter
11 months ago

Maybe the multiplication symbol should be replaced with "x" automatically in the slugs? We do this similarly for dashes which get replaced with hyphens. This was touched in the 6.9 release in #64089.

This could be tackled as part of #64151.

#2 in reply to: ↑ 1 @desrosj
11 months ago

  • Component General → Formatting
  • Keywords needs-patch added

Replying to westonruter:

Maybe the multiplication symbol should be replaced with "x" automatically in the slugs? We do this similarly for dashes which get replaced with hyphens.

I think this makes sense, @westonruter.

Tackling it with #64151 could make sense, but that seems like a much larger effort that may take a while. Since this is just one character and a one line change (excluding tests), the best path forward may be to fix this now rather than wait.

#3 @westonruter
11 months ago

  • Milestone Awaiting Review → 7.0

#4 @westonruter
11 months ago

  • Description modified (diff)
  • Summary Disallow Multiplication Symbol in WordPress Titles → Convert times/multiplication symbol (×) into the letter "x" when sanitizing titles for slugs

#5 @iflairwebtechnologies
11 months ago

  • Keywords has-patch added; needs-patch removed
  • Version → trunk

Hi
This patch resolves issue #64284, where the multiplication symbol (×) wasn't being replaced with the letter "x" when sanitizing titles for slugs.

The patch adds a line in the sanitize_title_with_dashes function to replace the multiplication sign (×) with "x" directly, as well as its encoded form (%c3%97).

Please review the patch and let me know if any changes are required.

#6 follow-up: @palak678
11 months ago

I tested this on 6.9 RC3 with PHP 8.4. The title cleanup works well now. The multiplication sign changes to a normal ‘x’ in the slug. This makes the slug function easier to understand and more consistent.

#7 in reply to: ↑ 6 @iflairwebtechnologies
11 months ago

Replying to palak678:

I tested this on 6.9 RC3 with PHP 8.4. The title cleanup works well now. The multiplication sign changes to a normal ‘x’ in the slug. This makes the slug function easier to understand and more consistent.

Are you asking whether the patch we created (64284.diff) is functioning correctly, or if any improvements are needed?

#8 @desrosj
11 months ago

  • Version trunk

The Version field is for indicating the version of WordPress that first experienced a given problem.

Please leave the version as set unless you’ve discovered details that show it was introduced during a different release. Switching back to trunk since this was not introduced during the 6.9 release.

#9 @palak678
11 months ago

Replying to @iflairwebtechnologies

I tested this on 6.9 RC3 with PHP 8.4, and without applying the patch the multiplication sign is already converting to a normal ‘x’ in the slug. The functionality seems to be working.

Last edited 11 months ago by palak678 (previous) (diff)

#10 @westonruter
10 months ago

  • Keywords needs-patch added; has-patch removed

#11 @westonruter
10 months ago

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

The patch 64284.diff​ seems to have unrelated changes.

Please open a pull request for collaboration and testing.

Also, I'm not entirely sure that sanitize_title_with_dashes() is the right function to apply this change in, since it is not related to replacing characters with dashes (read: hyphens). This is with discussing. That said, it actually does already replace some characters to 'x':

<?php
// Convert &times to 'x'.
$title = str_replace( '%c3%97', 'x', $title );

So this seems to be already settled.

The relevant addition from the patch then seems to be:

<?php
// Add the new line to convert the multiplication sign (×) directly to "x"
$title = str_replace( '×', 'x', $title );

Still, the patch replaces wp_is_valid_utf8() with seems_utf8() without explanation.

This will also need unit tests.

This ticket was mentioned in ​PR #10659 on ​WordPress/wordpress-develop by ​@abcd95.


10 months ago
#12

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

#13 @abcd95
10 months ago

Since there has been some time since the last activity on this ticket, I took the liberty of raising a PR for better collaboration and added comprehensive unit tests to cover the complete behavior with an enhanced patch.

  • Replaced ×, &times;, and &#215; with 'x' before URL encoding (as suggested by @westonruter)
  • Added test methods covering all forms and contexts

Please take a look and let me know if it looks good.

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


7 months ago

#15 @audrasjb
7 months ago

  • Keywords needs-testing added

As per today's bug scrub: The patch looks fresh and needs testing. Let's keep it in the milestone for now.

@westonruter as you commented the PR, are you now fine with this implementation?

#16 @ozgursar
7 months ago

I couldn't reproduce the multiplication sign × issue with the latest trunk 7.0-beta2-61752-src on PHP 8.2.29.

Could it be due to the fix for the #19820?

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


7 months ago

#18 @audrasjb
7 months ago

  • Milestone 7.0 → 7.1

As per today's 7.0 pre-RC1 bug scrub:

We have 1 unsuccessful reproduction report. Punting to 7.1.

#19 @gaisma22
7 months ago

  • Keywords dev-feedback close added; needs-testing removed

Reproduction Report

Environment

  • WordPress: 7.0-beta6-62085-src
  • PHP: 8.3.30
  • Server: nginx/1.29.6
  • Database: MySQL 8.4.8
  • Browser: Brave
  • OS: Ubuntu
  • Theme: Twenty Twenty-Five 1.4
  • MU Plugins: None
  • Plugins: None (first test), Hello Dolly 1.7.2 (second test)

Steps taken

  1. Navigated to Posts -> Add New Post.
  2. Entered "2×2 Test Post" as the post title using the multiplication symbol x (U+00D7).
  3. Published the post.
  4. Checked the permalink and slug field after publishing.
  5. Slug shows "2x2-test-post" - x correctly converted to "x".
  6. Reset environment and repeated with Hello Dolly 1.7.2 active.
  7. Same result - slug shows "2x2-test-post".

Result: ❌ Bug is not occurring

Expected behavior

Based on the ticket, the multiplication symbol x (U+00D7) in a post
title should be stripped from the slug, producing "22-test-post"
instead of "2x2-test-post".

Additional Notes

  1. Tested first in a clean environment with no plugins active.
  2. Environment was reset and the test was repeated with Hello Dolly 1.7.2 active.
  3. Both tests produced the same result.
  4. This matches comment #16 by @ozgursar who could not reproduce on 7.0-beta2. The fix may have been applied as part of #19820.
  5. Removing needs-testing as the bug does not reproduce in WordPress 7.0-beta6-62085-src. Adding close.

Screenshots/Screencast with results

Before Publishing:
https://i.ibb.co/pvnNvQCd/2-2-Test-Post-before-publish.png

After Publishing:
https://i.ibb.co/fzFKnrRN/2-2-Test-Post-after-published.png

#20 @wildworks
2 months ago

@himanshupathak95, Could you check if ​PR 10659 is still valid or if all issues have been resolved in trunk?

#21 @abcd95
2 months ago

Hi @wildworks! Thanks for checking in.

I investigated this against the latest trunk, and IMO the PR is still valid.

While it's true that the raw multiplication symbol (×) now correctly converts to x in trunk (thanks to an old fix from #19820 that handles %c3%97 after URL encoding), trunk still fails on HTML entities.

If a user or an external importer uses &times; or &#215; in the title (e.g., 2&times;2), trunk completely strips the entity out during slug generation, resulting in 22 instead of 2x2.

This PR explicitly handles all three formats (×, &times;, &#215;)

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


2 months ago

#23 @adrianduffell
2 months ago

  • Keywords early added
  • Milestone 7.1 → Future Release

This was discussed in today's bug scrub. It has a PR ready for review but I’d like to punt this since RC 1 is due in a few days time. It looks valuable to fix soon, so tagging with early for consideration in 7.2.

Note: See TracTickets for help on using tickets.