🇺🇸United States @recrit

attached is a backport patch for 6.0.x for anyone that has not updated to 7.0.x yet.

MR 105 created with the following:
- Maintains the same intended solution as the original - find the element with "data-bef-auto-submit-delay" and attach listeners to it.
- Finds the correct parent with "[data-bef-auto-submit-full-form]" even when there are intermediate elements in between "[data-bef-auto-submit]" and "[data-bef-auto-submit-full-form]".
- Changed the "$form" variable to "$needsAutoSubmit" since not all elements found with the selectors are forms.

attaching patch from 🐛 Schema App should not deploy on error pages Fixed

recrit → created an issue.

MR 38 created from the patch #37 ✨ Add option to not upscale images that are too small to fit target scale Needs review . The patch 37 can be used for any composer builds and applies to the latest 2.x.

the attached patch adds the following:
- AJAX controller updates only from 🐛 Facets with AJAX not working in most of situations Needs review to properly set the session on the new request. The other changes in 3052574 conflict with this patch since it is trying to fix the JS and change how the blocks are rendered in the AJAX controller.

- AJAX controller updates from 🐛 New $request created in FacetBlockAjaxController missing ajax_page_state RTBC to persist the "ajax_page_state".

static patch for safe composer builds.

@joseph.olstad using the automatically generate patch is dangerous since it is actively updated. A static patch is preferred to avoid any issues with new code pushed to the MR

posting a static patch for composer builds

The change can cause JS errors for $form is null when there is no delay found on the element or parent. This can happen when the front end theme has an extra element in between the `.views-exposed-form` top level DIV and the child FORM element.

-        let $form = $(e);
+        const $delay = $(e).data('bef-auto-submit-delay') ? $(e) : $(e).parent().data('bef-auto-submit-delay');
+        const $form= $delay ? $(e).parent() : null;

when there is no delay set, the $form is null here causing errors.

I created MR 145 with the paragraphs-3315835-2.patch.

@camhoward thanks for the summary of how to use 2.x. The site that I am working on does not use Google Analytics, just GTM. It seems that this can only be accomplished with the 1.x version of the module at this point.

Proposed Solution:
Update "drupal-rector/src/Drupal10/Rector/Deprecation/AnnotationToAttributeRector.php" with the following to detect single argument plugins with a missing key.
The patch attached for palantirnet/drupal-rector applies the following update:

    /**
     * @param array|ArrayItemNode[] $parsedArgs
     *
     * @return Attribute
     */
    private function createAttribute(string $attributeClass, array $parsedArgs): Attribute
    {
        $fullyQualified = new FullyQualified($attributeClass);
        // Support attributes with a single value constructor.
        // For example: '@ViewsField("my_custom_views_field")'.
        if (count($parsedArgs) === 1 && empty($parsedArgs[0]->key)) {
          $parsedArgs[0]->key = 'id';
        }

        $args = [];
       ...

recrit → created an issue.

it appears that composer just ignores any patched changes to the composer so you may still need the following to be able to remove the "drupal/variationcache" dependency:

    "replace": {
      "drupal/variationcache": "*"
    },

ignore 3421610-9-do-not-test.patch, i left a comman in the composer.json.
the attached patch also removes the dependency from composer.json. This patch is for 8.x-1.6 with the package info.

the attached patch also removes the dependency from composer.json. This patch is for 8.x-1.6 with the package info.

@msnassar The Undefined variable $resource_url should be fixed in the latest MR.
I created an MR against 11.x with error message updated to the following. I removed the "|" from the message since system logs often are delimited with "|" which could cause issue with parsing the log.

      $this->logger->error(
       'An error occurred while fetching an oEmbed resource for Media ID: %media_id, Embed URL: %media_url, oEmbed Resource URL: %resource_url. Error: @error',
        [
          '%resource_url' => !empty($resource_url) ? $resource_url : 'unknown',
          '%media_url' => $media_url,
          '%media_id' => $media->id(),
          '@error' => $e->getMessage(),
        ]
      );

Attached is a static patch of the MR for composer builds. Please contrib to the MR for any further development.

recrit → changed the visibility of the branch 10.4.x to hidden.

recrit → changed the visibility of the branch 10.3.x to hidden.

recrit → changed the visibility of the branch 11.x to hidden.

recrit → changed the visibility of the branch 3202896-dont-display-oembed to hidden.

Cleaner patch for 10.3.x with no tests

Attached is static patch of the MR 7350 for 11.x and a patch adapted for 10.3.x

In general, dev work should be done on the MR 5385 for the latest 11.x branch since this incorporates all the discussion thus far in this issue. Once approved, the changes would be back ported to any older branches.

Attached are static patches that can be used for composer builds.

• Patch for 11.x only: drupal-11.x-MR-5385--20240827-1.patch
• Patch for 10.3.x, 11.x: drupal-11.x-10.3.x-MR-5385--without-phpstan-changes--20240827-1.patch - This is the MR 5385 for 11.x without the phpstan changes so that it can be applied to 10.3.x also.

MR 3 created to rename the config file.
Attached is a static patch of the MR for composer builds.

recrit → created an issue.

The MR diff cannot be applied to a stable tag (i.e. 8.x-1.12) due to the changes in tests/modules/entity_composite_relationship_test/entity_composite_relationship_test.info.yml. The attached patch does not include any tests changes so that it can be used in a composer build.

MR 233 created. Attached is a static patch for composer builds.

recrit → created an issue.

thanks everyone. i committed this to the latest 3.x branch https://git.drupalcode.org/project/paragraphs_type_help/-/commit/470eb91...

Adding static patch with the PHP 8.3 fix.

Closing as I just realized the get_class code is added in the patch in https://www.drupal.org/project/access_unpublished/issues/3335950 ✨ Move Access Tokens UI to an entity Local Task / Tab Needs work

recrit → created an issue.

Adding static patch the MR9 for composer builds.

adding static patch of the MR for composer builds

@joevagyok it has been a minute since I created this patch, but I thought changing the class to "error" when it was enforced handled the "hard limit" OR am I missing something?

        if ($this.hasClass('maxlength_js_enforce')) {
          options['enforce'] = true;
          options.cssExceeded = 'error';
        }

attached a 11.x static patch for composer builds

the following MR's have been updated per the MR feedback on MR 5438:
11.x: MR 5437
10.1.x: MR 5438

updated the static patch with latest changes to fix issue with JS/CSS aggregation. This sets library file weights to the same as core so that files can be replaced with the jquery_ui contrib module files.

Moved to needs work since there is an issue with the latest patch when using core's asset aggregation.

updated the static patch with latest changes.

updated static patch with latest changes.

MR15 has been re-based with the latest 8.x-1.x. Attached is a static patch of MR15 at commit 5eb1e13b.

@KarlShea
Regarding your comment "I added most of #145 except for removing image_path_flush() in Media, that will affect every type not just OEmbed."
- That seems like an edge case. You would need to have another media type that is using the thumbnail file of an oEmbed resource. The normal usage would be a 1:1 of media to thumbnail file.

Regarding your comment "Edit: actually I removed #145, it looks like there was some disagreement."
There still needs to be a solution for busting cache - browser, edge cache, reverse proxy.

Options so far for cache busting:
1. The thumbnail generation approach as used in patch #145
Pros:
- Busts through all layers of caching - browser, edge cache reverse proxy.
- Automatically deletes the old file and any image styles for the old file.
- ???
Cons:
- A call to "getMetadata" for the "thumbnail_uri" assumes that the thumbnail is being set on the media entity. If it does not update the media, then this would cause deletion of the old file.
- ???

2. Add a modified timestamp URL query parameter as described in #146 and #147
Pros:
- Does not alter the thumbnail generation.
- ???
Cons:
- URL query parameters are not always respected by all layers of caching. For example, edge caching in Akamai could ignore the query parameter for some assets such images depending on the configuration in Akamai.
- Depending on the approach to get the modified time, you could be incurring a file system operation ("filemtime") on every image_style theme rendering.
- ???

@nghua please provide an interdiff from your patch to the latest so that it is obvious what you have changed. Your patch does not address all of the issues with patch #7 that were fixed in patch #9. I un-hid patch #9 until #10 has been updated and an interdiff provided.

Note: the latest MR 3037 does not have the browser cache busting that was added in #145 ✨ Expose triggering update of media metadata + thumbnail to end users Needs work .

To me the approach in this issue is better than ✨ Improve oEmbed exception logging Needs work .

✨ Improve oEmbed exception logging Needs work : The change in core/modules/media/src/OEmbed/ResourceFetcher.php will now expose a lot of information to an anonymous user when the fetcher error message is displayed by \Drupal\media\Plugin\media\Source\OEmbed::getMetadata(). That is not desired on any website.

  throw new ResourceException('Could not retrieve the oEmbed resource: ' . $e->getMessage(), $url, [], $e);

The following change in core/modules/media/src/OEmbed/ResourceFetcher.php will now expose a lot of information to an anonymous user when the fetcher error message is displayed by \Drupal\media\Plugin\media\Source\OEmbed::getMetadata(). That is not desired on any website.

  throw new ResourceException('Could not retrieve the oEmbed resource: ' . $e->getMessage(), $url, [], $e);

In my opinion, the approach on 🐛 Don't display OEmbed error to anonymous visitors when resource stops being available Needs work is a better solution. It does not display the error to anonymous users (or any user that cannot edit the media) and provides a reference to the Media ID causing the issue.

updated static patch with latest changes.

updated static patch with latest changes.

See 🐛 Remove duplicate jQuery UI JavaScript and CSS files Needs review that has a fix for all core libraries by replacing the individual files instead of adding a library dependency which can cause resolved library weighting issues.

I created an MR with code in the library alter to replace the actual files in the core libraries.
I attached a static patch to be used for builds only. Please use the issue branch for any updates to the patch.

I am having this same issue with multiple duplicate JS and CSS files loaded on the page.
This issue may be a duplicate of 🐛 Module defines jQuery UI files independently from Core. Can cause CSS priority issues in AJAX calls Needs review , however that ticket focused on CSS and there is no changes in the open MR as of 2024-06-03.

Duplicate JS and CSS files cause performance issues since it bloats the aggregated files and can cause other issues as noted in 🐛 Module defines jQuery UI files independently from Core. Can cause CSS priority issues in AJAX calls Needs review .

For example, I am seeing the following duplicate JS files:

modules/contrib/jquery_ui/assets/vendor/jquery.ui/ui/widgets/autocomplete-min.js?v=1.13.2
core/assets/vendor/jquery.ui/ui/widgets/autocomplete-min.js?v=10.2.6

core/assets/vendor/jquery.ui/ui/keycode-min.js?v=10.2.6
modules/contrib/jquery_ui/assets/vendor/jquery.ui/ui/keycode-min.js?v=1.13.2

core/assets/vendor/jquery.ui/ui/widgets/menu-min.js?v=10.2.6
modules/contrib/jquery_ui/assets/vendor/jquery.ui/ui/widgets/menu-min.js?v=1.13.2

modules/contrib/jquery_ui/assets/vendor/jquery.ui/ui/safe-active-element-min.js?v=1.13.2
core/assets/vendor/jquery.ui/ui/safe-active-element-min.js?v=10.2.6

modules/contrib/jquery_ui/assets/vendor/jquery.ui/ui/unique-id-min.js?v=1.13.2
core/assets/vendor/jquery.ui/ui/unique-id-min.js?v=10.2.6

modules/contrib/jquery_ui/assets/vendor/jquery.ui/ui/version-min.js?v=1.13.2
core/assets/vendor/jquery.ui/ui/version-min.js?v=10.2.6

modules/contrib/jquery_ui/assets/vendor/jquery.ui/ui/widget-min.js?v=1.13.2
core/assets/vendor/jquery.ui/ui/widget-min.js?v=10.2.6

For example, I am seeing the following duplicate CSS files:

modules/contrib/jquery_ui/assets/vendor/jquery.ui/themes/base/autocomplete.css
core/assets/vendor/jquery.ui/themes /base/autocomplete.css

modules/contrib/jquery_ui/assets/vendor/jquery.ui/themes/base/core.css
core/assets/vendor/jquery.ui/themes/base/core.css

modules/contrib/jquery_ui/assets/vendor/jquery.ui/themes/base/menu.css
core/assets/vendor/jquery.ui/themes/base/menu.css

modules/contrib/jquery_ui/assets/vendor/jquery.ui/themes/base/theme.css
core/assets/vendor/jquery.ui/themes/base/theme.css

Changing this back to needs work since there is nothing to compare in the MR.

The fix in the MR worked for me. I set to "Needs work" since this needs tests for the missing width and height keys.

@DieterHolvoet You had added a "ImageFieldItemList.php" list class to fix an issue that you mentioned in #45 🐛 Using partially synchronized image fields causes validation errors and php warnings Needs work .
There is test coverage for the scenario that you mentioned in `ImageOnTranslatedEntityTest::testSyncedImagesWithTranslatableProperties` and `ImageOnTranslatedEntityTest::testSyncedImagesWithTranslatablePropertiesAndContentModeration`.
If I set the list handler back to `FileFieldItemList`, then the tests still pass.

Please test with setting the list handler back to `FileFieldItemList` to see if the current updates fix your issue without the need for a new list class.
If a new list class is still needed, then we will need test coverage for the issue that you see.

attaching a static patch for composer builds

recrit → created an issue.

For url provided "https://music.youtube.com/watch?v=r8rfB0ZDqlg", you could use Drupal core media's oEmbed "Remove Video" which will detect that Youtube url.
oEmbed works for YouTube Music Podcasts too, but the embed url is not readily available.
Example:
https://music.youtube.com/playlist?list=PLSome-ThingRandom
would be detected if entered as:
https://www.youtube.com/embed/videoseries?list=PLSome-ThingRandom

successfully rebased with the latest 11.x, moving back to RTBC

"Needs Review Queue Bot" rejection - this was caused by 11.x updating the ImageItem plugin definition to use PHP attributes instead of annotations.
I'm working on rebasing to fix the conflicts.

attached a static patch of the current MR that can be used with 2.1.x for the library dependency change adding "ckeditor5/internal.drupal.ckeditor5" similar to 3.x.

Hiding patch 15 to avoid confusion. The MR should be used for any fixes.

I had a custom selection plugin that extended Drupal\views\Plugin\EntityReferenceSelection\ViewsSelection. Since the plugin ID was not "views", the detection to remove the element validation (unset($sub_handler_settings['view']['#element_validate']); would not run, resulting in silent form error and the filter config values not saving.

I changed views handler detection to class based instead of plugin id to support any plugin that extends the ViewsSelection class.

Attaching a static patch of MR 4053 for builds to use. This patch applies to 11.x and 10.2.5.

I have ran into this multiple times as well.
Common Use Case: An editor wants to unpublish all translations of a node so that the page is no longer visible to anonymous traffic for any language.

Can a core maintainer weigh in on the logic behind this?
Form description: "Only this translation is published. You must publish at least one more translation to unpublish this one."
Code comment in "core/modules/content_translation/src/ContentTranslationHandler.php":

      // If there is only one published translation we cannot unpublish it,
      // since there would be nothing left to display.

- "nothing left to display": that is the goal for the use case mentioned above. Users that can view unpublished content will still be able to view it.

MR rebased to 11.x, tests have been updated to not use the root user.

@Berdir thank you! that makes sense now. I'll switch it to add permissions to the test user.

@smustgrave I have rebased the branch with 11.x. The new test "ImageOnTranslatedEntityTest::testSyncedImagesWithTranslatablePropertiesAndContentModeration" is now failing for a very odd issue - the root user 1 cannot access "fr/node/1/edit". This was not an issue prior to the rebase so my thinking is something changed in 11.x. The root user should not be denied, so it is very odd. I've tried a few code changes to the tests, but none have worked.

@smustgrave All MR comments have been resolved and the Issue Summary has been updated.

the patch in #2 🐛 New structure for template_preprocess_field_multiple_value_form Needs work fixed the issue for me.

Failing Test for MR 5385: core/modules/image/tests/src/Kernel/ImageItemTest.php:137

    1) Drupal\Tests\image\Kernel\ImageItemTest::testImageItem
    Failed asserting that null matches expected 88.
    
    /builds/issue/drupal-3218426/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
    /builds/issue/drupal-3218426/core/modules/image/tests/src/Kernel/ImageItemTest.php:137
    /builds/issue/drupal-3218426/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

This test explicitly sets the width to NULL before re-saving the image. The width does not get calculated in the new ImageItem::onChange() method since it does nothing when the changed property is either "width" or "height" allowing others to set the values. The original ImageItem code would calculate the width and height in ImageItem::preSave() if either the width or height is missing.

Updates completed:
- Created a method to ensure that the width and height are set - ImageItem::ensureImageDimensions().
- Used the new method in ImageItem::onChange() and ImageItem::preSave().

The attached patch can be used for composer builds.

Some cleanup - All MRs and patches should be based on 11.x. For that reason, I hid MR 1373 and all patches on this ticket so that the focus can be on MR 5385.

MR 5385:
- Based on 11.x.
- has the changes from the original patch.
- has the setValue approach from patch #40 🐛 Using partially synchronized image fields causes validation errors and php warnings Needs work created by @Berdir. This approach keeps the values if they already exist so that the width and height does not need re-calculated, see #38 🐛 Using partially synchronized image fields causes validation errors and php warnings Needs work for more details.
- has a new ImageFieldItemList::hasAffectingChanges() added by @DieterHolvoet , see #45 🐛 Using partially synchronized image fields causes validation errors and php warnings Needs work for more details.

Failing Test for MR 5385: core/modules/image/tests/src/Kernel/ImageItemTest.php:137

    1) Drupal\Tests\image\Kernel\ImageItemTest::testImageItem
    Failed asserting that null matches expected 88.
    
    /builds/issue/drupal-3218426/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
    /builds/issue/drupal-3218426/core/modules/image/tests/src/Kernel/ImageItemTest.php:137
    /builds/issue/drupal-3218426/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

recrit → changed the visibility of the branch 3218426-using-content-moderation to hidden.

the "Proposed resolution" in the issue summary has been updated to reflect the final solution

PENDING WORK

1. DONE - Need a change record to be created and trigger_error updated. See Draft CR at https://www.drupal.org/node/3438992 →
2. PENDING - Needs test coverage of trigger_error.

Cause of test failure:
Failed test: Drupal\Tests\image\Functional\ImageAdminStylesTest::testPreviewImageShowInPrivateScheme
The following code that was added in 🐛 Image styles - thumbnails are broken in config page when private file system is used RTBC must be after the hook_file_download has been called since that hook is expected a URI and not a file path.

    // If it is default sample.png, ignore scheme.
    if ($image_uri === $sample_image_uri) {
      $image_uri = $target;
    }

Fix implemented:
- Moved the code that removes the scheme from $image_uri to after the hook_file_download call.
- Added a check for $image_uri !== $sample_image_uri when checking whether to test the converted image uri.

The https://www.drupal.org/files/issues/2024-03-01/3096570-22.patch → patch applies cleanly to 11.x. I have created the MR 7350 to move this ticket along.
All patches on this issue have been hidden. The MR should be used for any updates.

PENDING WORK - see https://www.drupal.org/project/drupal/issues/3096570#comment-14954234 🐛 Redirect correct language page after node save Needs work

1. Need a change record to be created and trigger_error updated
2. Needs test coverage of trigger_error.

@leo liao thanks for the 10.2 patch. Any code fixes are applied to the latest dev branch - 11.x.

Some more debugging: The changes in [#3127116} to target the sample images is causing this test failure. This change was committed in February 2024 and it removes the scheme when the sample images are requested. See commit fcc1ba6.

Just noticed that this still needs tests

Debugging now, this may be related to this issue since the failure is for a private scheme test "Drupal\Tests\image\Functional\ImageAdminStylesTest:testPreviewImageShowInPrivateScheme"