🇺🇸United States @Chris Burge

Using the get() and set() methods on SectionComponent will result in the setting being stored in the additional property because a hidden property doesn't exist. ✨ Support third party settings for components within a section Needs work seeks to replace the additional property with third-party settings. I'd recommend adding a hidden property to the SectionComponent class to avoid creating technical debt.

@zebda You'd need to change the media source of the media type, which core doesn't allow: #2928256: Users shouldn't be able to change the media source plugin after the media type is created → .

That said, when I look at the commit, I'm seeing the only change was in the MediaTypeForm class. I don't see anything from stopping you from changing the media source programmatically. You might even be able to make the change by updating the media type's config entity in core's configuration management UI (e,g. /admin/config/development/configuration/single/export and /admin/config/development/configuration/single/import). Definitely test in non-prod first.

A number of tests are failing with the following output:

Test code or tested code printed unexpected output:
Deprecated: Optional parameter $key_value_factory declared before required parameter $module_handler is implicitly treated as a required parameter in /builds/issue/oembed_providers-3528538/src/OEmbed/ProviderRepositoryDecorator.php on line 116
Deprecated: Optional parameter $logger_factory declared before required parameter $module_handler is implicitly treated as a required parameter in /builds/issue/oembed_providers-3528538/src/OEmbed/ProviderRepositoryDecorator.php on line 116

The ProviderRepository constructor change in D10, and the $key_value_factory and $logger_factory parameters are no longer optional: __construct() in D10 vs __construct() in D9.

D9 is no longer supported by this module, so all we need to do is make the parameters required:

  public function __construct(ProviderRepositoryInterface $decorated, EntityTypeManagerInterface $entity_type_manager, ClientInterface $http_client, ConfigFactoryInterface $config_factory, TimeInterface $time, $key_value_factory, LoggerChannelFactoryInterface $logger_factory, ModuleHandlerInterface $module_handler, $max_age = 604800) {

Needs work to address PHPCS failure:

FILE: src/Form/SecuritytxtSignForm.php
------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
 79 | WARNING | Do not concatenate strings to translatable strings, they should be part of the t() argument and you
    |         | should use placeholders (Drupal.Semantics.FunctionT.ConcatString)
----------------------------------------------------------------------------------------------------------------

chris burge → changed the visibility of the branch 3375732-text-on-sign to hidden.

Manual testing is successful:

1. Ensure no signature is stored in config.
2. Load /.well-known/security.txt.sig
3. Observe a 200 response with an empty body.
4. Deploy merge request.
5. Load /.well-known/security.txt.sig
6. Observe a 400 response with the standard Page Not Found page.

Opened an MR with patch 3383742-handle-empty-signature-1.patch.

chris burge → made their first commit to this issue’s fork.

chris burge → created an issue.

greggles → credited chris burge → .

@niral098 - The header JS is inserted with hook_page_attachments(), so I'd recommend looking for other implementations of this hook that may be overwriting the value of html_head. Re the footer JS, that's inserted by decorating the html_response.attachments_processor service, and this has been problematic in some instances. I'd recommend looking for other services decorating the html_response.attachments_processor service.

@programeta - "it's not working as expected" - What's not working? Is the header JS not being inserted consistently? Is the footer JS not being inserted consistently? Are you seeing errors in the console? We need to know what's not working in order to consider your feedback.

Setting to 'Needs work' due to random test failures:

Pass: https://git.drupalcode.org/issue/memcache-2996615/-/jobs/5137611
Fail: https://git.drupalcode.org/issue/memcache-2996615/-/jobs/5137778

Following up from #50/51 here. (@btully - Thank for your those testing steps!)

Steps to reproduce:

1. Install vanilla Drupal with Standard profile
2. Enable Memcache and switch cache backends to cache.backend.memcache for bootstrap, discovery, config, and default cache bins.
3. Add a hook_entity_update() function with sleep(2); to emulate real-world entity save.
4. Create Basic Page node (node/1) with content "Chris Test" and save.
5. Observe "Chris Test" on node view page.

Here's the hook_entity_update() code:

function slow_save_entity_update(\Drupal\Core\Entity\EntityInterface $entity) {
  sleep(2);
}

Next, I configured JMeter with a simple load test - 10 concurrent requests with random query strings appended to the request URL.

1. Start JMeter load test.
2. Load node/1/edit and update text to "Chris Test 2" and save.
3. Observe "Chris Test 1" still displays on node view page.
4. Load node/1/edit and observe "Chris Test 1" still displays on the edit page.
5. Save the node without making any edits.
6. Observe "Chris Test 2" now displays.

Next, I deployed the MR code and retested:

1. Deploy MR code and set the aforementioned cache bins to use the cache.backend.memcache_transaction_aware cache backend.
2. Start JMeter load test.
3. Load node/1/edit and update text to "Chris Test 3" and save.
4. Observe "Chris Test 3" now displays on the node view page.
5. Load node/1/edit and observe "Chris Test 3" is displayed there as well.
6. No stale content is served.

This testing indicates that the MR is effective.

chris burge → created an issue.

+1 for this addition to core. In the meantime, you can also accomplish this with an event subscriber:

namespace Drupal\my_module\EventSubscriber;

use Drupal\views\Ajax\ViewAjaxResponse;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

/**
 * Change the scroll-to-top behavior for the my_view view.
 */
class AjaxResponseEventSubscriber implements EventSubscriberInterface {

  /**
   * {@inheritdoc}
   *
   * @return array
   *   The event names to listen for, and the methods that should be executed.
   */
  public static function getSubscribedEvents() {
    return [
      KernelEvents::RESPONSE => 'alterCommands',
    ];
  }

  /**
   * React to ResponseEvent.
   *
   * @param \Symfony\Component\HttpKernel\Event\ResponseEvent $event
   *   Response event.
   */
  public function alterCommands(ResponseEvent $event) {
    $response = $event->getResponse();

    if ($response instanceof ViewAjaxResponse
      && $response->getView()->id() === 'my_view'
      ) {

      $commands = &$response->getCommands();
      foreach($commands as $key => $command) {
        if ($command['command'] == 'scrollTop') {
          unset($commands[$key]);
        }
      }
      $commands = array_values($commands);
    }
  }

}

In my case, I wanted to modify the selector to the views' .view-content class, which is only a minor change:

      $commands = &$response->getCommands();
      foreach($commands as $key => $command) {
        if ($command['command'] == 'scrollTop') {
-         unset($commands[$key]);
+         $commands[$key]['selector'] = $command['selector'] . ' .view-content';
        }
      }
-     $commands = array_values($commands);
    }

It'd be worthwhile to consider allowing a site builder to set the scrollTop selector instead of adding a toggle. It would meet my use case, which is necessitated by responsive design (where the exposed filters stack atop the content on mobile).

@vamirbekyan - Thank you for filing this ticket. Are you able to provide steps to reproduce?

As the maintainer for the Layout Builder Component Attributes modules, who also has contributed code to this issue, my vote is to commit. It'll be up to contrib to handle upgrade paths. The code provided by @luke.leber is what I'm planning on using. (@luke.leber THANK YOU for sharing that code btw.) It's performant and can handle a large number of records. Modules using the additional property will have until D12 to address the deprecation.

@alexpott - Thank you for the bug report and for the MR code. It turns out phar-io/version is a Drupal core dev dependency. I'm kind of surprised no one has reported this until now.

@almunnings - That's a good catch. I updated the MR.

chris burge → created an issue.

Duplicate? 🐛 Media library allows adding videos from providers other than Youtube/Vimeo using Media embed Needs work

Duplicate? 🐛 Media Fails to Enforce oEmbed Allowed Providers Setting Active

MR82 allows HTML is all tags. If the maintainer favors the second solution, I'd be happy to update the MR.

On the latest commit, I set the decoration_priority to -100 so that our new_relic_rpm.html_response.attachments_processor decorator gets called earlier.

There are a handful of contrib modules that decorate core's html_response.attachments_processor service. In the case of Acquia's Site Studio module, the module decorates the core service but then breaks the decorator pattern in its implementation. As a result, no decorators with a higher weight are called. (While the current decoration_priority is 100, it was defaulted to 0 in versions earlier than 8.x-8.0.2 of the Site Studio module.)

Note: We may be able to get rid of the HtmlResponseAttachmentsProcessorDecorator code when ✨ Allow additional keys in #attached Active lands in core.

I'd missed that addition. Here's a link to the D.O. documentation: https://www.drupal.org/docs/develop/automated-testing/performance-tests → .

It looks like we'd probably use the getQueries() method and/or the ->getQueryCount() method on the PerformanceData class.

Updated issue summary.

I don't see a way to "add" test coverage for this performance optimization. Existing test coverage should ensure we haven't broken anything and should cover the changes made by the MR. As of last run, tests are passing.

chris burge → created an issue.

I spoke too soon. I don't believe Memcache is at fault.

The SectionComponent class has never had a thirdPartySettings property. It is proposed in ✨ Support third party settings for components within a section Needs work ; however.

@hswong3i has your site previously run a patch from ✨ Support third party settings for components within a section Needs work ?

When a Layout Builder component is serialized for storage, it includes properties from its SectionComponent object. If a site is patched to add third-party settings to to LB components and that patch is later removed, you'll get the notice you posted. This is the same error without Memcache enabled:

Deprecated function: Creation of dynamic property Drupal\layout_builder\SectionComponent::$thirdPartySettings is deprecated in Drupal\Component\Serialization\PhpSerialize::decode() (line 21 of /var/www/html/web/core/lib/Drupal/Component/Serialization/PhpSerialize.php) 

I suspect if you search the database, you'll find orphaned thirdPartySettings properties inside serialized SectionComponent objects.

I'm running into the same notice in the issue description, and it's filling the logs every request. I can't explain it, but patch #4 fixes it. No more notices.

I missed this issue when opening ✨ Allow for RUM Manual Instrumentation Needs review , which implements this feature request in MR-5.

@godotislate - I like the approach of caching attachments in a class property. One thing I noticed when stepping through the method with a debugger is that editor_load() still gets called every loop if the text format doesn't have an editor assigned to it.

I just pushed a commit to the MR that add an $editors property to the class and checks if the text format has been processed yet instead of checking if $settings['editor']['formats'][$format_id] is set. This ensures editor_load() is only called once for each text format.

I'm not sure what kind of text coverage could be added for this MR. We can't test performance with unit tests, and the existing test coverage should ensure we haven't broken anything.

Provider docs: https://support.bluebillywig.com/advanced-topics/oembed-service/
oEmbed specs: https://oembed.com/

https://*.bbvms.com/p/*/c/*.html?inheritDimensions=true&placementOption=default appears to be a valid resource URL.

The provider endpoint URL, as specified by bbvms.com is https://<publication>.bbvms.com/oembed/; however, the endpoint URL cannot contain wildcards. I'm not sure how you'd do this at runtime off the top of my head. You might need to decorate the media.oembed.resource_fetcher service with custom code and modify the ResourceFetcher::fetchResource() method to modify the URL before the resource is fetched.

@sea2709 - That's exactly the issue. We can't change Html::cleanCssIdentifier without causing CSS bugs.

#13

I'm wondering if the best way to solve this issue is to use a configuration flag/setting similar to the $conf['allow_css_double_underscores'] setting in D7 that allowed BEM double underscore CSS selectors.

I think the behavior should be configurable. Maybe a $setting to define additional characters?

@senzaesclusiva - Glad to hear it!

chris burge → created an issue.

@erwangel - This was fixed in 🐛 Calling static trait method is deprecated (redux) Fixed but hasn't been released yet.

@senzaesclusiva - To clarify - Is the issue resolved for you?

I'm not seeing any changes in 2.0.0 that explains the exception. We do have two prior reports of issues with Rumble.com; however. Please see #3253757-14: Unable to create the remote video → .

@mabho Could you try rebuilding cache?

chris burge → created an issue.

chris burge → created an issue.

Closing in favor of ✨ Add Drupal 11 Support; Drop support for 'ckeditor' and 'ckeditor4to5upgrade' plugin types Active /

chris burge → created an issue.

@justcaldwell - Thanks for opening this issue!

On second thought, we can just change the trait to a class.

It looks like we'll need to remove HelperTrait and just define a procedural function in the module file.

🐛 Allow form redirects to ignore ?destination query parameter Fixed was committed and released in D10.2.0: https://www.drupal.org/node/3375113 →

Here's the code I used to attempt to implement the fix:

function layout_builder_operation_link_form_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {
  if ($form_state->getFormObject() instanceof \Drupal\layout_builder\Form\OverridesEntityForm) {
    $form_state->setIgnoreDestination();
  }
}

I confirmed with my IDE that ->setIgnoreDestination() is firing; however, the destination query parameter is still being rendered. I don't believe the "fix" addresses the use case for operation links. I think we might just need to update the comment in layout_builder_operation_link_preprocess_links() to remove the reference to the core issue we were waiting on.

Committed and released

D11 support was already added in 📌 Add Drupal 11 Support Fixed - it just needs released.

Manual testing was successful. Logs are clean.

@aaron.ferris - Thanks for jumping in and providing a solution!

chris burge → created an issue.

This issue is blocked by 📌 Drupal 11 compatibility for Features module Active .

chris burge → made their first commit to this issue’s fork.

chris burge → made their first commit to this issue’s fork.

chris burge → made their first commit to this issue’s fork.

chris burge → made their first commit to this issue’s fork.

chris burge → created an issue.

I'm good committing this. Let's release as 2.2.0.

Drupal 11 compatibility is released in 2.2.0 → .

Access checks are only necessary on content entities, not on config entities: https://www.drupal.org/node/3201242 → .

chris burge → made their first commit to this issue’s fork.

When I asked around among colleagues today, I was pointed to this blog post: https://dev.acquia.com/blog/drupalelementstyle-add-styles-drupal-media-c....

@bkosborne - Is this solution functionally equivalent to the plugin you authored?

This makes sense. I have a customer who leverages paragraphs for their site, often with a dozen or more rich text fields on given node edit page. Page load times are quite slow in these instances.

The ::getAttachments() method belongs to the Drupal\editor\Plugin\EditorManager class, so moving this issue to editor.module.

Static caching seems reasonable. &drupal_static() can be used. Editors and text formats have a 1:1 relationship. Text formats have permissions and control access their editor, so I don't see any permission issue here with caching.

We'll want to make sure to preserve the behavior of hook_editor_js_settings_alter() (i.e. $this->moduleHandler->alter('editor_js_settings', $settings); and below should not be cached.

Needed to run tests. Maintainers should feel free to remove .gitlab-ci.yml file prior to merging and merge 📌 Switch from Drupal CI to GitLab CI Needs review separately.

@_m - Code is merged! Thanks

Chris Burge → created an issue.

@neclimdul - Sorry for flooding your inbox, but I think we're really close on this MR now.

In terms of outstanding items, there's been discussion around how to describe the settings' behaviors:
https://git.drupalcode.org/project/new_relic_rpm/-/merge_requests/5#note...

I'm open to revising documentation and relabeling. It strikes me the machine name 'auto' could be better named 'no-action'.

There was also discussion regarding the UpdateTest test class:
https://git.drupalcode.org/project/new_relic_rpm/-/merge_requests/5#note...

@phenaproxima - Thanks for the code review

Chris Burge → created an issue.

Tests are passing again - now to address the rest of the technical review.

There's too much going on in this MR to not be running tests. I added .gitlab-ci.yml. The maintainers should feel free to remove the file and commit separately in 📌 Switch from Drupal CI to GitLab CI Needs review before merging.

I'd recommend implementing hook_entity_operation_alter() in custom code.

Chris Burge → made their first commit to this issue’s fork.

There is some interaction between Drupal's internal cache system and newrelic_get_browser_timing_footer() that I can't explain. I'm logged in as user 1. If I load any admin page, the footer JS is consistently rendered. If I visit non-admin pages, the footer JS is only rendered on the first page load after a cache rebuild. I even tried disabling the decorator and acting on the markup directly in an event subscriber, acting on the Symfony ResponseEvent. Stashing the footer JS in the default cache bin seems to do the trick, however.