Make WordPress Core

Opened 5 years ago

Closed 6 days ago

Last modified 6 days ago

#41208 closed defect (bug) (fixed)

Dashboard News & Events widget does not properly handle entities (e.g. en/em dash)

Reported by: nickciske Owned by: audrasjb
Milestone: 6.0 Priority: low
Severity: normal Version: 4.8
Component: Administration Keywords: has-patch commit
Focuses: ui, javascript Cc:

Description

"WordCamp Minneapolis – St. Paul, Minnesota, USA" is displayed as "WordCamp Minneapolis – St. Paul, Minnesota, USA"

Attachments (8)

Screen Shot 2017-06-29 at 2.02.53 PM.png (45.1 KB) - added by nickciske 5 years ago.
Screenshot of issue
WP News and Announcements OK at-sign.PNG (38.9 KB) - added by kpegoraro 5 years ago.
Screenshot of at sign in WP News and Events
WP News and Announcements OK em-dash.PNG (68.3 KB) - added by kpegoraro 5 years ago.
WP News and Events em-dash example
WP News and Announcements OK en-dash.PNG (57.3 KB) - added by kpegoraro 5 years ago.
WP News and Announcements OK en-dash example screenshot
41208.diff (667 bytes) - added by sayedulsayem 3 months ago.
This is documented function of lodash to convert string from escaped string.
41208.2.diff (657 bytes) - added by sayedulsayem 3 months ago.
padrone me for my previous patch. This is the actual working solution
events-widget-entities-before.png (11.4 KB) - added by sabernhardt 2 weeks ago.
character codes showing before patch
events-widget-entities-after.png (9.4 KB) - added by sabernhardt 2 weeks ago.
character codes decoded with patch

Download all attachments as: .zip

Change History (28)

@nickciske
5 years ago

Screenshot of issue

#1 @kpegoraro
5 years ago

On Chrome on Windows, I see the same result for the Minneapolis event in the first screenshot from nickciske, but I do see em- and en-dashes and at-signs properly for other events (screenshots attached).

@kpegoraro
5 years ago

Screenshot of at sign in WP News and Events

@kpegoraro
5 years ago

WP News and Events em-dash example

@kpegoraro
5 years ago

WP News and Announcements OK en-dash example screenshot

#2 @iandunn
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Owner set to iandunn
  • Priority changed from normal to low
  • Status changed from new to accepted

I'm seeing this too. I haven't had time to really dig into it, but the – entity is returned by the w.org API, which seems appropriate at first glance.

My first guess would be that {{ is preventing the entity from being rendered. Running the data through htmlspecialchars_decode() before it gets returned from get_events() might be a good fix. I think that'd be safe, since it'd still be passed through {{, but we'd need to verify that.

#3 @shedonist
4 years ago

I think this issue may have been resolved by Meetup itself. I tried editing one of my future meetups (https://www.meetup.com/WordpressDevSeacoast/events/243889267/ - on 1/11/18) to contain an endash or an emdash and it shows properly both in Meetup and also when I looked at it in the dashboard (search for "Nottingham") and the "Users — TBD" meetup shows fine.

Steps: What I did was paste the entity directly into the meetup title. Then I waited for the widget to refresh itself (I deleted my local transients, but it seems like Meetup or something up the chain was caching the titles for a bit). I am able to find my meetup at the moment by searching for "Nottingham" (this narrows the search so I can see further ahead to the Jan meetup). Once the updated "Users — TBD" title appeared in the widget, it was displaying correctly.

#4 @iandunn
4 years ago

  • Owner iandunn deleted
  • Status changed from accepted to assigned

#5 @iandunn
6 months ago

#53851 was marked as a duplicate.

#6 @Hareesh Pillai
5 months ago

  • Milestone changed from Future Release to 5.9

Adding it to the upcoming major milestone for consideration. A similar issue was faced recently with Santa Catarina - Brazil.

@sayedulsayem
3 months ago

This is documented function of lodash to convert string from escaped string.

@sayedulsayem
3 months ago

padrone me for my previous patch. This is the actual working solution

#7 @sayedulsayem
3 months ago

  • Keywords has-patch added; needs-patch removed

#8 @sayedulsayem
3 months ago

  • Focuses ui javascript added

#9 @sabernhardt
2 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.9 to 6.0

From ticket:53851#comment:6 :

I agree w/ Jake about keeping the escaping, but ticket:41208#comment:2 might work.

#10 @audrasjb
2 weeks ago

Justr noting that the patch proposed above doesn't work in most case since unescape() only handles a small amount of entities (source: http://underscorejs.org/#unescape).

#11 @audrasjb
2 weeks ago

Sames goes for htmlspecialchars_decode (source: https://www.php.net/manual/fr/function.htmlspecialchars-decode.php).
I think we need to use html_entity_decode.

This ticket was mentioned in PR #2124 on WordPress/wordpress-develop by audrasjb.


2 weeks ago

  • Keywords needs-refresh removed

#13 @audrasjb
2 weeks ago

The above PR fixes the issue.

## How to test

Step 1: simulate the issue

Using the PR above, replace this line:
$event['title'] = html_entity_decode( $event['title'] );
with:
$event['title'] = 'test – testing' );

See that it displays the raw HTML entity on the dashboard.

Step 2: fix the issue

Now replace the line with:
$event['title'] = html_entity_decode( 'test – testing' );

See that it displays: test – testing.

#14 @audrasjb
2 weeks ago

  • Owner set to audrasjb

@sabernhardt
2 weeks ago

character codes showing before patch

@sabernhardt
2 weeks ago

character codes decoded with patch

#15 @sabernhardt
2 weeks ago

For testing, I wanted to keep both the original code and the patch changes intact, so I edited the $events array immediately at the beginning of the trim_events() function. (The PR diff applied changes around this.)

$events[0]['title'] = '“test – testing” …';
$events[1]['title'] = 'Users — TBD';
 $events[1]['type'] = 'wordcamp';
$events[2]['title'] = '« L’Événement » à Nîmes';

#16 @audrasjb
2 weeks ago

Thanks for testing @sabernhardt.
So according to your test case, I assume we're good to go with this patch?

#17 @sabernhardt
2 weeks ago

This patch probably is ready, though I did not add the commit keyword in case someone else would want to test additional characters and/or add feedback.

The only identified problems occur with the dash character codes, and the PR fixes those plus others (both numeric codes and entities).

#18 @audrasjb
6 days ago

  • Keywords commit added

Thanks @SergeyBiryukov, @ocean90, @sabernhardt for the review 👍

#19 @audrasjb
6 days ago

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

In 52608:

Administration: Properly handle HTML entities in the News & Events dashboard widget.

This change adds support for various HTML entities in the News & Events dashboard widget.

Props nickciske, kpegoraro, iandunn, shedonist, sayedulsayem, sabernhardt, audrasjb, SergeyBiryukov, ocean90.
Fixes #41208.

Note: See TracTickets for help on using tickets.