WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 8 weeks ago

#54232 closed defect (bug) (fixed)

Unbreakable spaces not escaped

Reported by: jdy68 Owned by: audrasjb
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-screenshots has-patch commit
Focuses: javascript, administration Cc:

Description

Hey there,

I spotted an message that has a bad handling of non-breaking spaces in the interface of the image editing feature, see screencapture message-modifications-media.png. The translate of this string is correct.

This message corresponds to the following string (https://translate.wordpress.org/projects/wp/dev/admin/fr/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=32033&filters%5Btranslation_id%5D=88460924) located on line 252 of image-edit.php file (https://build.trac.wordpress.org/browser/trunk/wp-admin/includes/image-edit.php?marks=252#L252).

See screenshot: message-edit-media.png

For other similar messages, the consideration of non-breaking spaces is correct, here are two examples:

Example 1:
the string in translate: https://translate.wordpress.org/projects/wp/dev/admin/fr/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=10187821&filters%5Btranslation_id%5D=84172065
its location: https://build.trac.wordpress.org/browser/trunk/wp-admin/js/nav-menu.js?marks=1423#L1423
See screenshot: message-delete-menu.png

Example 2:
the string in translate: https://translate.wordpress.org/projects/wp/dev/admin/fr/default/?filters%5Bstatus%5D=either&filters%5Boriginal_id%5D=76269&filters%5Btranslation_id%5D=51383668
its location: https://build.trac.wordpress.org/browser/trunk/wp-admin/themes.php?marks=226#L226
See screenshot: message-delete-theme.png

Thanks for the fix.

Attachments (5)

message-delete-menu.png (8.3 KB) - added by jdy68 2 months ago.
message-edit-media.png (13.2 KB) - added by jdy68 2 months ago.
message-delete-theme.png (5.8 KB) - added by jdy68 2 months ago.
Capture d’écran 2021-10-08 à 00.21.28.png (29.7 KB) - added by audrasjb 2 months ago.
Works for me using Chrome
54232.patch (484 bytes) - added by mukesh27 2 months ago.
Patch.

Download all attachments as: .zip

Change History (16)

#1 @jdy68
2 months ago

  • Summary changed from Uunbreakable spaces not escaped to Unbreakable spaces not escaped

#2 @sebastienserre
2 months ago

Hello @jdy68

I've made the following test:
+ replacing the string on line 252
<?php _e( "There are unsaved changes that will be lost. 'OK' to continue, 'Cancel' to return to the Image Editor." ); ?>
with

<?php _e( 'There are unsaved changes that will be lost. "OK" to continue, "Cancel" to return to the Image Editor.' ); ?>

+ Modifying the .po with POedit
+ Compiling the .mo with Poedit

Unfortunately, I always see the &nbsp in the message.

Last edited 2 months ago by sebastienserre (previous) (diff)

#3 @sebastienserre
2 months ago

  • Focuses administration added

@audrasjb
2 months ago

Works for me using Chrome

#4 follow-up: @audrasjb
2 months ago

  • Component changed from Media to Administration
  • Focuses javascript added
  • Keywords has-screenshots added
  • Owner set to audrasjb
  • Status changed from new to reviewing
  • Version 5.8 deleted

Thanks for the ticket,

Could you please share your browser version?
I tried to reproduce the issue and it works fine on my side using Chrome.

  • Removing version 5.8 as I don't think the issue was introduced in this version ;-)
  • Moving to component Administration as it looks like the bug will probably occur on each JavaScript confirm or alert box of the WordPress admin.
  • Adding Javascript focus

#5 in reply to: ↑ 4 ; follow-up: @SergeyBiryukov
2 months ago

  • Milestone changed from Awaiting Review to 5.9

Replying to audrasjb:

Moving to component Administration as it looks like the bug will probably occur on each JavaScript confirm or alert box of the WordPress admin.

I think the issue is about that specific string in wp-admin/includes/image-edit.php. As noted in the ticket description, other similar strings are displayed correctly.

I was able to reproduce the issue with that string in the latest Microsoft Edge version using these steps:

  • Switch to the fr_FR locale.
  • Open an image in Media Library for editing.
  • Make some changes and click the Cancel button.
  • Note the &nbsp; entities in the message.

The string is retrieved for displaying using the jQuery .html() function in wp-admin/js/image-edit.js. Replacing .html() with .text() fixes the issue in my testing.

#6 in reply to: ↑ 5 @mukesh27
2 months ago

Replying to SergeyBiryukov:

I was able to reproduce the issue with that string in the latest Microsoft Edge version using these steps:

  • Switch to the fr_FR locale.
  • Open an image in Media Library for editing.
  • Make some changes and click the Cancel button.
  • Note the &nbsp; entities in the message.

The string is retrieved for displaying using the jQuery .html() function in wp-admin/js/image-edit.js. Replacing .html() with .text() fixes the issue in my testing.

I reproduce the issue with the same step and replacing .html() with .text() fixes the issue in my test.

Let me add patch so other can test it.

@mukesh27
2 months ago

Patch.

#7 @mukesh27
2 months ago

  • Keywords has-patch added; needs-patch removed

#8 @audrasjb
2 months ago

Thanks for clarifying @SergeyBiryukov and for the patch @mukesh27, it looks good to me 👍

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


8 weeks ago

#10 @audrasjb
8 weeks ago

  • Keywords commit added

Reviewed during today's bug scrub.

Adding commit keyword.

Last edited 8 weeks ago by audrasjb (previous) (diff)

#11 @SergeyBiryukov
8 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 51907:

Media: Display the unsaved changes dialog in image edit form using jQuery .text() function.

This ensures that HTML entities like non-breaking spaces are properly displayed instead of being encoded.

Props jdy68, sebastienserre, audrasjb, mukesh27, SergeyBiryukov.
Fixes #54232.

Note: See TracTickets for help on using tickets.