Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#54232 closed defect (bug) (fixed)

Unbreakable spaces not escaped

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

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 5 years ago.
message-edit-media.png (13.2 KB ) - added by jdy68 5 years ago.
message-delete-theme.png (5.8 KB ) - added by jdy68 5 years ago.
Capture d’écran 2021-10-08 à 00.21.28.png (29.7 KB ) - added by audrasjb 5 years ago.
Works for me using Chrome
54232.patch (484 bytes ) - added by mukesh27 5 years ago.
Patch.

Download all attachments as: .zip

Change History (16)

#1 @jdy68
5 years ago

  • Summary Uunbreakable spaces not escapedUnbreakable spaces not escaped

#2 @sebastienserre
5 years 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

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

Version 0, edited 5 years ago by sebastienserre (next)

#3 @sebastienserre
5 years ago

  • Focuses administration added

@audrasjb
5 years ago

Works for me using Chrome

#4 follow-up: @audrasjb
5 years ago

  • Component MediaAdministration
  • Focuses javascript added
  • Keywords has-screenshots added
  • Owner set to audrasjb
  • Status newreviewing
  • Version 5.8

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
5 years ago

  • Milestone Awaiting Review5.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
5 years 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
5 years ago

Patch.

#7 @mukesh27
5 years ago

  • Keywords has-patch added; needs-patch removed

#8 @audrasjb
5 years 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.


5 years ago

#10 @audrasjb
5 years ago

  • Keywords commit added

Reviewed during today's bug scrub.

Adding commit keyword.

Last edited 5 years ago by audrasjb (previous) (diff)

#11 @SergeyBiryukov
5 years ago

  • Resolutionfixed
  • Status reviewingclosed

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.