Skip to content

Handle breaking changes in jQuery 4's AJAX prefilter#1612

Merged
Chouby merged 14 commits into
masterfrom
fix-1527
Jan 24, 2025
Merged

Handle breaking changes in jQuery 4's AJAX prefilter#1612
Chouby merged 14 commits into
masterfrom
fix-1527

Conversation

@Screenfeed
Copy link
Copy Markdown
Contributor

@Screenfeed Screenfeed commented Jan 10, 2025

Fixes #1527. Probably also fixes #1603.

jQuery 4 introduces a breaking change in the AJAX prefilter.

@manooweb explained it well in this comment. The TL;DR being: in jQ 3 the data is stringified before the prefilter, while in jQ 4 it is done after. Then we must expect several types of data in our prefilter.

The solution suggested here is to deal with the given data type without changing it (or changing it to a similar type):

  • undefined, null, String => string.
  • A plain Object => Object.

Code moved

The current file admin.js is enqueued only in our settings pages and the wizard, so it has been renamed into settings.js.
The new JS code has been moved to a dedicated JS file ajax-filter.js, then imported in a new file admin.js, which is enqueued in all admin screens. In the future we expect to re-use ajax-filter.js in PLL Pro by creating a new JS package containing all the common code (for now we will duplicate the file).

Code linted

PHPCS suddenly started linting the new file settings.js while it wasn't linting the old file admin.js.

  1. phpcbf has been used to add some missing space characters.
  2. Each insertion of HTML code has been verified, to make sure the inserted code is safe. Then those errors have been ignored and comments have been added to explain why.

Manual tests

php callback using data in $_POST

print_r( $_POST );
die();
$.ajax() config Data received by the php callback Supported Broken
{ data: [7, "a"] } Array( [undefined] => ) No
{} Array( [pll] => 1 ) No
{ data: null } Array( [pll] => 1 ) No
{ data: "" } Array( [pll] => 1 ) No
{ data: "A string" } Array( [A_string] => null, [pll] => 1 ) No
{ data: "foo=2" } Array( [foo] => 2, [pll] => 1 ) No
{ data: {foo: "bar"} } Array( [foo] => bar, [pll] => 1 ) No

Note

The first case doesn't work because the first argument passed to Object.assign() is an Array.

php callback using data in request's body

print_r( file_get_contents( 'php://input' ) );
die();
$.ajax() config Data received by the php callback Supported Broken
{ data: "A string", contentType: "text/plain; charset=UTF-8" } A string&pll=1 ⚠️ Yes
{ data: document.implementation.createDocument( null, "books" ), contentType: "application/xml; charset=UTF-8", processData: false } ) <books/> No

Caution

The first case is problematic (however I don't think this is a regression).

Note

The second case doesn't work because the data is not a plain object.

php callback using JSON data in request's body

print_r( json_decode( file_get_contents( 'php://input' ), true ) );
die();
$.ajax() config Data received by the php callback Supported Broken
{ data: JSON.stringify( [7, "a"] ), contentType: "application/json; charset=UTF-8" } ) Array( [0] => 7, [1] => a ) No
{ data: JSON.stringify( {foo: 2} ), contentType: "application/json; charset=UTF-8" } Array( [foo] => 2, [pll] => 1 ) No

Note

The first case doesn't work because the first argument passed to Object.assign() is an Array.

Warning

A similar work must be done in Polylang Pro.

Testing

We can use Test jQuery Updates plugin to replace current jQuery version (3.7.1) with the current jQuery beta version (4.0.0-beta.2). Deactivate the plugin to return back to 3.7.1 version.

To test with third party plugin refer to older PRs:

Note

There is also ajax requests in ACF. For example by using our related post block from our PLL ACF Blocks plugin. The ajax request is sent when selecting the block to fill ACF fields.

Caution

The new code should still working with jQuery 3.7.1

@Screenfeed Screenfeed requested a review from manooweb January 10, 2025 10:45
@Screenfeed Screenfeed self-assigned this Jan 10, 2025
@Screenfeed Screenfeed added this to the 3.7 milestone Jan 10, 2025
@Screenfeed Screenfeed requested a review from manooweb January 17, 2025 14:50
@Hug0-Drelon Hug0-Drelon self-requested a review January 22, 2025 08:35
Comment thread admin/admin-base.php Outdated
Comment thread js/src/admin.js Outdated
Comment thread js/src/lib/ajax-filter.js Outdated
Comment thread js/src/admin.js Outdated
Comment thread js/src/lib/ajax-filter.js Outdated
Comment thread modules/wizard/wizard.php
@Chrystll
Copy link
Copy Markdown

Chrystll commented Jan 22, 2025

✅ Here's how to test with Polylang only for now:

  1. Activate the Test jQuery Updates plugin.

  2. Perform actions that generate AJAX requests in the back office interacting with Polylang. Here are some examples:

    • /wp-admin/ => related to the "We have noticed that you have been using Polylang for some time. We hope you love it, and we would really appreciate it if you would give us a 5-star rating."
    • /wp-admin/edit.php => visit this page (error). Manipulate the top language admin switcher.
    • /wp-admin/post-new.php
    • /wp-admin/post.php?post=257&action=edit
    • /wp-admin/edit-tags.php?taxonomy=category => change the language in "Sets the language."
    • /wp-admin/edit-tags.php?taxonomy=category&post_type=post&from_tag=53&new_lang=de => action "add new category."
      • autocomplete field when you want to link two categories by translation.
    • /wp-admin/admin.php?page=mlang
    • /wp-admin/admin.php?page=mlang_strings => use the filter group.
    • /wp-admin/admin.php?page=mlang_settings => manipulate the module.
    • /wp-admin/admin.php?page=mlang_wizard => add a license, add a language, apply language for untranslated content
  3. Open the Developer Tools console and look for trim() is not a function errors.

  4. If the error appears, it is likely related to string processing in Polylang with jQuery 4.

  5. Once the error related to jQuery 4 is reproduced, test the fix-1527.

  6. Does the fix-1527 resolve the jQuery 4 errors shown in the console?
    ✅Yes

Note:
On /wp-admin/admin.php?page=mlang_wizard&step=languages, I don’t see in the console that I'm using WordPress jQuery: 4.0.0-beta.2.

✅ Here's how to test with Polylang Pro and ACF Pro:
I fetched and applied the dev-fix-1527 branch for testing in Polylang Pro. I tested the AJAX requests with ACF Pro, for example, when switching the Field Type, it loads the Translations field of Polylang Pro with the default choice—test successful, no more trim() is not a function errors in the console and the Translations field is well loaded as expected.

✅ Ninja Forms with Polylang:
No more trim() is not a function errors in the console when duplicating or deleting a form from /wp-admin/admin.php?page=ninja-forms.

✅WooCommerce Tree Table Rate Shipping v.1.36.1
No more trim() is not a function errors in the console when adding a rule from /wp-admin/admin.php?page=wc-settings&tab=shipping&section=tree_table_rate&trs_global

✅FS License Manager v.4.3.5
No more trim() is not a function errors in the console when adding a license from /wp-admin/admin.php?page=license-manager-add-license-key

@manooweb
Copy link
Copy Markdown
Contributor

manooweb commented Jan 22, 2025

Note: On /wp-admin/admin.php?page=mlang_wizard&step=languages, I don’t see in the console that I'm using WordPress jQuery: 4.0.0-beta.2.

It isn't the case on my side:

image

image

Note

jQuery is only here on these steps: Licenses, Languages and Content.

@Chouby Chouby merged commit 8ccbdf0 into master Jan 24, 2025
@Chouby Chouby deleted the fix-1527 branch January 24, 2025 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Javascript exception in addPolylangParametersAsString for options.data=null trim() is deprecated in the forthcoming jQuery 4

5 participants