Make WordPress Core

Opened 7 weeks ago

Closed 8 days ago

#62713 closed defect (bug) (fixed)

Policies accordion does not work for plugin names with non-English characters

Reported by: ecgan's profile ecgan Owned by: audrasjb's profile audrasjb
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Privacy Keywords: has-patch
Focuses: ui, javascript, administration, privacy Cc:

Description

In the Privacy > Policy Guide page (/wp-admin/options-privacy.php?tab=policyguide), there is a Policies accordion that shows plugins that call wp_add_privacy_policy_content.

When the plugin name is translated and contains non-English characters, the accordion does not expand for the plugin.

For example, I am using WooCommerce Points and Rewards plugin. When I change my user profile language to "Français", the plugin name in the Policy Guide page is displayed as "Points et récompenses". Clicking on it does not expand the accordion. In the browser console, an error shows up that says:

Uncaught Error: Syntax error, unrecognized expression: #privacy-settings-accordion-block-points-et-r%c3%a9compenses
    at find.error (jquery.js?ver=3.7.1:1487:8)
    at tokenize (jquery.js?ver=3.7.1:2153:8)
    at select (jquery.js?ver=3.7.1:2612:20)
    at Function.find (jquery.js?ver=3.7.1:939:9)
    at Function.<anonymous> (jquery-migrate.js?ver=3.4.1:266:17)
    at obj.<computed> [as find] (jquery-migrate.js?ver=3.4.1:181:20)
    at jQuery.fn.init.find (jquery.js?ver=3.7.1:2822:11)
    at jQuery.fn.init (jquery.js?ver=3.7.1:2932:32)
    at obj.<computed>.<anonymous> (jquery-migrate.js?ver=3.4.1:226:17)
    at new obj.<computed> (jquery-migrate.js?ver=3.4.1:181:20)

Attachments (3)

wordpress-privacy-policy-guide-accordion-not-working.png (88.9 KB) - added by ecgan 7 weeks ago.
Privacy policy guide accordion not working because of non-English character in plugin name.
62713.diff (760 bytes) - added by sabernhardt 7 weeks ago.
removes % characters from $sanitized_policy_name
62713-increment.diff (1.7 KB) - added by sabernhardt 7 weeks ago.
using an increment instead of the plugin name for each section ID

Download all attachments as: .zip

Change History (17)

@ecgan
7 weeks ago

Privacy policy guide accordion not working because of non-English character in plugin name.

#1 @ecgan
7 weeks ago

The jQuery JavaScript code that finds the accordion block does not work because the element ID contains characters that needs to be escape.

See: How do I select an element by an ID that has characters used in CSS notation?

WordPress is calling it like below, which is causing the JavaScript console error:

jQuery('#privacy-settings-accordion-block-points-et-r%c3%a9compenses')

If we do the escaping like below, then it would work as expected:

jQuery('#privacy-settings-accordion-block-points-et-r\\%c3\\%a9compenses')

Alternatively, we may also try this without jQuery:

document.getElementById('privacy-settings-accordion-block-points-et-r%c3%a9compenses')​

#2 @ecgan
7 weeks ago

I believe the plugin names should be translated using __() function - that's how WordPress is calling wp_add_privacy_policy_content too: https://github.com/WordPress/wordpress-develop/blob/17b50d3d9c0c3f1383bae3c9241bd618c9160b09/src/wp-admin/includes/class-wp-privacy-policy-content.php#L704

Last edited 7 weeks ago by ecgan (previous) (diff)

#3 @sabernhardt
7 weeks ago

  • Component changed from General to Privacy
  • Milestone changed from Awaiting Review to 6.8
  • Version 6.7.1 deleted

Hi and thanks for the report!

This apparently has been possible at least as early as 6.0.

Finding a plugin that breaks the button functionality is easier when the profile language does not have any Latin characters (for example, WP Content Copy Protection & No Right Click in Arabic).

@sabernhardt
7 weeks ago

removes % characters from $sanitized_policy_name

#4 @sabernhardt
7 weeks ago

  • Keywords has-patch added

The $sanitized_policy_name simply needs to create a unique ID for each expandable section. If the ID strips any % characters, that should remain unique.

If someone else prefers editing the script, that would be fine. (I tend to avoid editing scripts if I can find another way.)

#5 @ecgan
7 weeks ago

Hi @sabernhardt, thanks for responding to my ticket and adding your idea and your patch here. I think that removing the % characters isn't really ideal as there might still be a chance to have conflict with other plugin names.

I would like to investigate this issue and contribute a fix to it when I can (but probably not so soon though), if somebody else doesn't beat me to it.

#6 @sabernhardt
7 weeks ago

  • Keywords needs-patch added; has-patch removed

Yes, the plugin names could conflict. I was not worried about having a plugin name that matches the modified URL encoding of another plugin, but #49916 and #51256 are about plugins that have exactly the same name.

Unfortunately, I do not find the plugin filename or slug coming from WP_Privacy_Policy_Content::get_suggested_policy_text(). Should the accordions simply use an increment instead of basing the ID on the plugin's information?

@sabernhardt
7 weeks ago

using an increment instead of the plugin name for each section ID

#7 @sabernhardt
7 weeks ago

  • Keywords has-patch added; needs-patch removed

I found an example for the increment in add_menu_classes() and followed that.

#8 @ecgan
6 weeks ago

Hi @sabernhardt, thanks for adding the links to previous tickets for more contexts. It shows that we can't rely on just plugin names (because there can be two identical plugins with same plugin names but in different plugin directories: activate-akismet/akismet.php and activate-akismet-old/akismet.php).

I think using an increment is a viable approach.

However, before we do that, I am also curious: why do we use plugin names in the first place? If we make the change from using plugin name to using an increment, will we break things?

I found that the use of plugin names / $sanitized_policy_name comes from the following links:

I don't see any reason mentioned for why we want to use plugin names. So, I think it may be okay to make the change to using an increment.

Should we include people from the above links into our discussion here?

Also, from the links above, the issue has been around since WordPress version 5.7.

#9 @sabernhardt
5 weeks ago

Should we include people from the above links into our discussion here?

Yes, @xkon might want to check this and/or give feedback.

#10 @audrasjb
9 days ago

I think the proposed implementation of a simple incremental ID is indeed the best way to solve the issue. I didn't found any situation where it was breaking the accordion behavior, using the plugins mentioned above in this ticket.

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


9 days ago
#11

#12 @audrasjb
9 days ago

@sabernhardt I added a pull request based on your patch to facilitate testing via WP Playground and to run Github Actions against this change (but I don't think we'll find any test failure with that patch).

@audrasjb commented on PR #8205:


9 days ago
#13

Works well on my side.

#14 @audrasjb
8 days ago

  • Owner set to audrasjb
  • Resolution set to fixed
  • Status changed from new to closed

In 59732:

Privacy: Replace Policy Name with an auto increment to avoid internationalized plugin name issues.

This changeset replaces plugin sanitized names with an auto increment integer to fix an issue with accordions displaying privacy policies for plugins with special characters in their names.

Follow-up to [50161].

Props ecgan, sabernhardt, audrasjb.
Fixes #62713.

Note: See TracTickets for help on using tickets.