- Issue created by @danielveza
- Merge request !3784Issue #3352549: Leftover D10 deprecations in ajax.js → (Open) created by danielveza
- Status changed to Needs review
over 1 year ago 12:56am 6 April 2023 - 🇳🇿New Zealand danielveza Brisbane, AU
Updated the deprecation to 11.0 following advice from @xjm
- Status changed to RTBC
over 1 year ago 2:31pm 6 April 2023 - Status changed to Needs work
over 1 year ago 7:31am 12 April 2023 - 🇫🇷France nod_ Lille
Let's take care of the todo in this issue too and add a deprecation warning. See https://www.drupal.org/node/3082634 →
* @todo Add deprecation warning after it is possible. For more information * see: https://www.drupal.org/project/drupal/issues/2973400
- Status changed to Needs review
over 1 year ago 11:13pm 12 April 2023 - 🇳🇿New Zealand danielveza Brisbane, AU
Added the deprecation errors, I think I've done it right by adding the returns but it would be good to get some eyes over it
- Status changed to Needs work
over 1 year ago 1:03pm 28 April 2023 - Status changed to Needs review
over 1 year ago 1:04pm 28 April 2023 - last update
over 1 year ago 29,108 pass, 124 fail The last submitted patch, 8: 3352549-8.patch, failed testing. View results →
- last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 29,108 pass, 124 fail - last update
over 1 year ago 29,108 pass, 124 fail - last update
over 1 year ago 29,108 pass, 124 fail - last update
over 1 year ago 29,108 pass, 124 fail - last update
over 1 year ago 29,108 pass, 124 fail - last update
over 1 year ago 29,108 pass, 124 fail - last update
over 1 year ago 29,108 pass, 124 fail - last update
over 1 year ago 29,108 pass, 124 fail - First commit to issue fork.
- last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 29,108 pass, 124 fail - First commit to issue fork.
- Status changed to Needs work
about 1 year ago 4:11am 6 November 2023 - 🇳🇿New Zealand quietone
There are two issues for this work, the other is 📌 Properly deprecate ajax.js methods Closed: duplicate . Although the other is older this one has an MR so, lets continue work here. Also, adding credir
- 🇬🇧United Kingdom catch
There are dozens of JavaScript test failures here, but from a quick scan, it looks like this deprecation might be all of them:
2x: Javascript Deprecation: The Drupal.theme.ajaxWrapperNewContent is deprecated in drupal:8.6.0 and is removed from drupal:11.0.0. See https://www.drupal.org/node/2940704 2x in ViewsWizardTest::testCreateViewWizard from Drupal\Tests\views_ui\FunctionalJavascript ---- Drupal\Tests\workspaces\FunctionalJavascript\WorkspaceToolbarIntegration
- 🇬🇧United Kingdom longwave UK
The issue is this code, which triggers the deprecation on every Ajax insert:
// For backward compatibility, in some cases a wrapper will be added. This // behavior will be removed before Drupal 9.0.0. If different behavior is // needed, the theme functions can be overridden. // @see https://www.drupal.org/node/2940704 $newContent = Drupal.theme( 'ajaxWrapperNewContent', $newContent, ajax, response, );
I don't think the current deprecation is going to help anyway. The change notice tells you to override the method if you want to change the behaviour; if you do this then the deprecation we add here won't be fired - in fact it's only fired if you *don't* override it, which is the wrong way round?
- 🇬🇧United Kingdom longwave UK
Maybe what we need to do is:
- Remove
Drupal.theme.ajaxWrapperNewContent
andDrupal.theme.ajaxWrapperMultipleRootElements
- In
Drupal.AjaxCommand.insert
ifDrupal.theme.ajaxWrapperNewContent
exists then: - Issue a deprecation
- If
Drupal.theme.ajaxWrapperMultipleRootElements
exists then issue another deprecation - If
Drupal.theme.ajaxWrapperMultipleRootElements
doesn't exist, define it (because the first theme function might call it) - Call it as before
- Otherwise: just hardcode the wrapper in the multiple elements case? It's not clear from the change record what we will do if there are still more than one root-level element returned.
- Remove
- 🇬🇧United Kingdom catch
Or, decide this is too complicated, and just drop it all from Drupal 11 without any deprecations, as we deprecated this several years ago and the code mentions removal from Drupal 9 etc, so in the event anyone was overriding this they have had plenty of notice anyway.
This is very appealing and I think we should do it. We can always try to improve the 10.x deprecations if it turns out to be a problem, but it's also quite possible no-one is relying on it.
- 🇬🇧United Kingdom longwave UK
It's still not clear what to do about the multiple root elements case; this is the breaking change.
@lauriii in #736066-357: ajax.js insert command sometimes wraps content in a div, potentially producing invalid HTML and other bugs → :
The support for effects with multiple root elements is meant to be temporary. I also proposed that we give a warning for any users that would be affected by this logic, but we don't have a way to do that until #2962344: [policy, no patch] Document how to handle deprecations in JavaScript → has been resolve.
So if we only add the div when there's multiple root elements and an effect, the bug would be fixed in most cases, and there wouldn't be any known breaking changes for existing sites.
However, we still haven't triggered that warning, and there may be users relying on the div wrapper around multiple elements.
So, new proposal:
- In 10.3.x trigger a deprecation in
Drupal.theme.ajaxWrapperNewContent
only when the.length > 1
condition is true. If you were overriding this as per the change record, we assume you knew what you were doing; either you were adding divs back everywhere (but we told you this was going away), or you were overriding the multiple divs wrapper, and your code will be obsolete soon. - In 11.x remove all this code and drop the multiple div wrapper.
- In 10.3.x trigger a deprecation in
- 🇬🇧United Kingdom longwave UK
Rebased MR!3784 against 10.3.x and implemented #23.
- Status changed to Needs review
7 months ago 12:20pm 23 April 2024 - 🇬🇧United Kingdom longwave UK
Also opened MR!7670 for the removal in Drupal 11.
- Status changed to Needs work
7 months ago 12:45pm 23 April 2024 - First commit to issue fork.
- 🇺🇸United States DamienMcKenna NH, USA
The merge requests need to be updated to the latest changes on their respective branches.
- 🇷🇸Serbia finnsky
We need align with
https://www.drupal.org/project/drupal/issues/3238870 📌 Refactor (if feasible) use of jquery parseHTML function to use vanillaJS Needs work