Account created on 4 October 2012, over 11 years ago
#

Recent comments

🇬🇧United Kingdom Rob230

Patch #13 breaks order renewal because there is still a reference to the removed createOrder() function.

Here is the fix.

🇬🇧United Kingdom Rob230

Considering this breaks the core update page it's pretty serious.

Until there can be a new release, I have made a temporary patch (for Drupal core) to handle the situation.

🇬🇧United Kingdom Rob230

You can make it send any event using hooks.

🇬🇧United Kingdom Rob230

My problem was stage_file_proxy was capturing the requests and presumably fetching an old file from the production environment. I had to update the module (to resolve a bug) and then add css and js to excluded extensions.

🇬🇧United Kingdom Rob230

This doesn't work.

I have the change to nginx to pass CSS generation to Drupal (otherwise it has a 404).

The change discussed in this issue is added in Drupal 10.1. I can see it in AssetGroupSetHashTrait.php - it checks for the version being -1. The version is -1 if it's explicitly set to that, or if there is no version specified. I've stepped through it with Xdebug and it's hitting it, and adding a hash of the file contents as the version.

Yet, the generated CSS does not contain the CSS of the file. It's old and contains none of the new CSS that has been added.

I don't understand what to do any more. Since upgrading to Drupal 10 I can't get new CSS changes to appear. The changes exist in the source .css file, but never appear in the aggregated CSS generated by Drupal.

🇬🇧United Kingdom Rob230

We have the same problem when any module is enabled on the site - our deployments failed due to running out of memory. Disabling the admin_toolbar_tools module resolves the problem.

Our profiling found that having the admin_toolbar_tools module enabled means when a module is enabled, an extra 1.3 GB of memory is used and an extra 21 seconds of load time. Most of the memory is used by admin_toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks::getDerivativeDefinitions().

🇬🇧United Kingdom Rob230

I managed to get it working with these changes:

1. Put a blank file at vendor/simplesamlphp/simplesamlphp/modules/drupalauth

2. Apply the attached patch on top of MR12 (I have not added to your merge requests because I don't understand why it was done that way or how you got it working).

3. Update autoload section to composer.json as follows:

"autoload": {
    "psr-4": {
        "SimpleSAML\\Module\\drupalauth\\Auth\\": "www/modules/contrib/saml_idp/src/Auth/"
    },
    "classmap": [
        "scripts/composer/ScriptHandler.php",
        "www/modules/contrib/saml_idp/src/Auth/"
    ]
},
🇬🇧United Kingdom Rob230

Did anyone actually get this to work?

I'm trying the merge request. First of all, it does still need a drupalauth directory creating in vendor/simplesamlphp/simplesamlphp/modules, because the getModules() code scans that directory for modules, and it will say the drupalauth module doesn't exist without it.

But I get this error:

Caused by: Exception: Could not resolve 'drupalauth:External': The class 'SimpleSAML\Module\drupalauth\Auth\Source\External' isn't a subclass of '\SimpleSAML\Auth\Source'.

I don't exactly understand what this code in vendor/simplesamlphp/simplesamlphp/src/SimpleSAML/Module::resolveClass() is doing, but it seems like the www/modules/contrib/saml_idp/src/Auth/Source/External class is not being found, because it is a subclass of \SimpleSAML\Auth\Source.

🇬🇧United Kingdom Rob230

I think I am seeing this too, in Drupal 10. When running a migration, if it tries to output a message (e.g. an error), it causes the batch page to break due to a console error: wrapper is null.

static defaultWrapper() {
  let wrapper = document.querySelector('[data-drupal-messages]');
  if (!wrapper) {
    wrapper = document.querySelector('[data-drupal-messages-fallback]');
    wrapper.removeAttribute('data-drupal-messages-fallback');
    wrapper.setAttribute('data-drupal-messages', '');
    wrapper.classList.remove('hidden');
  }
  return wrapper.innerHTML === ''
    ? Drupal.Message.messageInternalWrapper(wrapper)
    : wrapper.firstElementChild;
}

There is no such element on the page with data-drupal-messages or data-drupal-messages-fallback attributes. I would guess the error was introduced in #77245: Provide a common API for displaying JavaScript messages .

🇬🇧United Kingdom Rob230

Confirmed that patch #57 applies to simple_gmap
3.1 and fixes the PHP 8.1 deprecation errors.

🇬🇧United Kingdom Rob230

I've seen this break on a number of D10 sites. Path in #84 fixes it.

🇬🇧United Kingdom Rob230

There are a few things wrong with merge request 6. The first is that it doesn't even apply by composer, although I can't see why. Something wrong with the diff generated by drupalcode? It rejects the changes to .info.yml even though they are simple changes that could easily be made, and it also creates a b directory and puts loads of files into it, when these are files that should be created.

If I checkout the merge request branch directly then I can get the changes, and it works for upgrading CKEditor and adding Tippy to the bar. But Tippy itself doesn't work. There is an error in tippy.js that tippy is undefined. I tried passing tippy in the closure but the code in tippy-bundle.umd.min.js actually depends on popperjs, which as far as I can tell doesn't exist.

The dependency for ckeditor/tippyjs is on core/popperjs which doesn't exist because it was removed in Drupal 10. I tried installing and enabling the popperjs module, and replacing core/popperjs with a dependency on the popperjs Drupal module and updating the library dependency. This resolved the console errors, but tippy still doesn't work.

When you select some text and choose the tippy toolbar item, it shows the dialog with text and tooltip options, both blank. Entering something in the tooltip box causes an error in ckeditor5-dll.js:5:597480 - Uncaught CKEditorError: view-writer-invalid-position-container. The source gets updated with this:

<tippy class="tippy-tooltip-text" data-tippy-content="[object Object]">

🇬🇧United Kingdom Rob230

Merge request !5867 doesn't apply to Drupal 10.2.2.

🇬🇧United Kingdom Rob230

Patch to add null coalescing operator. Casting to string would also work.

🇬🇧United Kingdom Rob230

I have this error in a view which is using search_api.

TypeError: Cannot assign null to property Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator of type string in Drupal\views\Plugin\views\argument\ArgumentPluginBase->unpackArgumentValue() (line 1310 of modules/views/src/Plugin/views/argument/ArgumentPluginBase.php).

Drupal\search_api\Plugin\views\argument\SearchApiStandard->fillValue() (Line: 160)
Drupal\search_api\Plugin\views\argument\SearchApiStandard->query() (Line: 1126)
Drupal\views\ViewExecutable->_buildArguments() (Line: 1282)
Drupal\views\ViewExecutable->build() (Line: 392)

It's a very simple view which just does a fulltext search and shows the search excerpt. It broke when upgrading to Drupal 10. Whilst patch #29 would fix it, I agree with #38 that the fix only treats the symptom but not the root cause. So I investigated some more and in my case I found it was due to a contextual filter which had a default value of a fixed value of an empty string, but which had a validator for "Content" that looked for the ID of a node and handles multiples with "One or more IDs separated by , or +".

So people should check the contextual filters and make sure an impossible situation hasn't been created where a blank value is validated. One could argue that the validator shouldn't cause an exception on the site, however Drupal had been instructed to do something that didn't make sense, and simply ignoring it would mean the mistake is never found.

🇬🇧United Kingdom Rob230

The patch #65 still breaks List (text) fields in views filters. Is nobody else having that problem?

🇬🇧United Kingdom Rob230

Oh that upgrade hook might have also been blocked by a block config which doesn't get updated by the patch. So I've changed that block config first and now it runs.

🇬🇧United Kingdom Rob230

I'm getting that error in the update hook that comes with 5.x (the D10 version of the module):

> [notice] Update started: context_update_8004
> [error] The "node_type" plugin does not exist. Valid plugin IDs for Drupal\Core\Condition\ConditionManager are: condition_group, config_pages_values_access, user_status, ... entity_bundle:paragraph
> [error] Update failed: context_update_8004
[error] Update aborted by: context_update_8004
[error] Finished performing updates.

Therefore, the post_update hook in patch #3 cannot ever run.

I've used the patch on D9.5 and used that as an intermediate deployment step before upgrading to D10 and context 5.x.

So should this be issue be for context 8.x-4.x so that it can run before D10? The change record is for Drupal 9.3.

🇬🇧United Kingdom Rob230

I'm sorry I don't have any ability to give an estimate as it is beyond my control.

See here: https://www.drupal.org/node/539608#s-application-review-timelines

🇬🇧United Kingdom Rob230

I have applied for permission to opt into security coverage.

🇬🇧United Kingdom Rob230

We've been using the module on a number of sites for a while now so I will create a stable release.

🇬🇧United Kingdom Rob230

Rob230 created an issue.

🇬🇧United Kingdom Rob230

No recursion protection if $build['#cache']['keys'] is not set, which can happen if the view mode is not cacheable, the entity is not the default revision, or the entity type is not render cacheable

This is causing most paragraphs to not be shown on our site, because Paragraph entity type is not render cacheable. What can be done about this? Should it be doing anything if $build['#cache']['keys'] is empty? Using an empty string as the $recursion_key seems to be a mistake.

🇬🇧United Kingdom Rob230

jeffschuler, yes we are using the merge request 4994 from that issue. Thanks for flagging this, I'll continue investigations in the other thread.

🇬🇧United Kingdom Rob230

We had a related problem after upgrading to D10 with Claro theme, the contents of the Chosen select was overflowing the box, but this patch #11 also fixes it.

🇬🇧United Kingdom Rob230

I am seeing the same thing. Have tried changing it to a single filter and then back to grouped but it still has the same behaviour.

The problem appears to be in here:

public function acceptExposedInput($input): bool {
  parent::acceptExposedInput($input);

  $this->value['center'] = [];

  $identifier = $this->options['expose']['identifier'];

  if (!empty($input[$identifier . '_center'])) {
    $this->value['center'] = $input[$identifier . '_center'];
  }

  return TRUE;
}

$this->value is "3" (a string) which presumably corresponds to the option I chose in the grouped filter. Changing the string value with array syntax causes an error and results in bizarre things happening (it changes into another string "A" but xdebug seems to think it is a reference to a class).

🇬🇧United Kingdom Rob230

It is affecting every paragraph, regardless of nesting, due to the problem I described (keys always being empty for paragraphs).

If it's not affecting other sites then I suppose there must be some other reason that the $build['#cache']['keys'] array is empty on my site, but I couldn't find one. I searched the code for anything mentioning the cache. Not sure what else to look for.

Do you know if my change would break anything? I can't see why it makes sense to try to detect recursion without any identifier. The only other way to fix this would be to stop $build['#cache']['keys'] from being empty but so far I couldn't figure out why that's the case - it looked intentional when I stepped through the code, maybe I need to dig deeper to see if it gets set later on and then removed.

🇬🇧United Kingdom Rob230

I don't know if I'm missing something here, but regardless of the reason there are no cache keys, surely a blank key should never be used to detect recursion. This patch shows the way I fixed it.

🇬🇧United Kingdom Rob230

It looks like the problem is this function:

public function setRecursiveRenderProtection(array $build): array {
  // Checks whether entity render array with matching cache keys is being
  // recursively rendered. If not already being rendered, add an entry to track
  // that it is.
  $recursion_key = implode(':', $build['#cache']['keys'] ?? []);
  if (isset($this->recursionKeys[$recursion_key])) {
    $this->getLogger('entity')
      ->error('Recursive rendering attempt aborted for %key. In progress: %guards', [
        '%key' => $recursion_key,
        '%guards' => print_r($this->recursionKeys, TRUE),
      ]);
    $build['#printed'] = TRUE;
  }
  else {
    $this->recursionKeys[$recursion_key] = $recursion_key;
  }
  return $build;
}

For paragraph entities, $build['#cache']['keys'] is empty, so $recursion_key is an empty string. This empty string then matches for every paragraph so they don't render.

From my investigations the reason why the keys array is empty for paragraphs is because of this code (also in EntityViewBuilder.php):

if ($this->isViewModeCacheable($view_mode) && !$entity->isNew() && $entity->isDefaultRevision() && $this->entityType->isRenderCacheable()) {
$build['#cache'] += [
  'keys' => [
    'entity_view',
    $this->entityTypeId,
    $entity->id(),
    $view_mode,
  ],
  'bin' => $this->cacheBin,
];

$this->entityType->isRenderCacheable() is FALSE for paragraphs because the Paragraph entity has render_cache = FALSE in its definition.

Simply checking if $recursion_key is a blank fixes this problem.

🇬🇧United Kingdom Rob230

This was my quick fix. I don't know if the span needs to be there for some reason - it has no CSS in the Seven theme.

🇬🇧United Kingdom Rob230

Another place it will appear is in the error message for a required field - even if the field itself displays correctly, the title (with suffix) gets escaped in the error message about the required field.

The best thing might be to remove the spans, because the #title of an element is used as a string in so many places.

🇬🇧United Kingdom Rob230

It does seem like it's way too aggressive. We have this unescaped HTML <span class="translation-entity-all-languages"> appearing at the end of many buttons and other form elements of a multilingual site.

For example, even something as simple as a link inside a container gets it added. The fix is to add '#multilingual' => TRUE to the container or the element. But the patch #5 will also fix it, and makes sense - what Drupal core is currently doing is incorrect because it is appending some HTML to a string that gets escaped.

🇬🇧United Kingdom Rob230

Hi Jerome, we have been using patch #8 for 9 months now and aren't getting the failed payment reports any more.

I'm surprised more people haven't reported this - broken 3DS2 implementation is pretty major. But it is up to the banks to reject and for small amounts most will allow without 3DS2. We were seeing almost every transaction over £50 be rejected by certain banks, and that no longer happens with patch #8.

Despite hundreds of people per month being rejected, only 1 or 2 of those would bother to email us to say it wasn't working, so perhaps a lot of site admins are unaware of the problem. You'd have to pay attention to the number of unsuccessful transactions in Braintree and consider it an issue.

🇬🇧United Kingdom Rob230

Why is the array cast to an object and then put inside an array? Why not just return the array?

🇬🇧United Kingdom Rob230

The point is to limit it from the server which provides the feed, not from the person making the request. If someone can request 50 and 50 would cause timeouts and pressure on the server then we should be able to stop that by setting a lower maximum.

🇬🇧United Kingdom Rob230

Also need this.

🇬🇧United Kingdom Rob230

It looks like the patches on #2988057: Voting API REST POST (create with REST) doesn't work, validation error, when referenced entity other than node do make this field work, so I can at least see the paragraph ID.

Unfortunately, there are no relationships that can be made from it (e.g. to the paragraph). There is just a "Content" ("The ID from the voted entity") relationship, which joins to node_field_data causing there to be no results.

🇬🇧United Kingdom Rob230

I'm seeing something similar (to the issue title anyway).

I have added a rate widget to paragraph entities, and voted.

You can see here that I have 2 votes, with entity IDs:

id	type	uuid	entity_type	entity_id	value	value_type	user_id	timestamp	vote_source	rate_widget
16	updown	1c5a19bc-3924-4d57-92e3-97515db64e38	paragraph	184	1	points	1	1694689401	a36fd09186fc87f08b70ee61bf531b1693a7a5315ae3daff0619756988da5311	like_widget
17	updown	d69d6d18-fc3d-498f-ad12-dc87232d651a	paragraph	126	1	points	1	1694689905	a36fd09186fc87f08b70ee61bf531b1693a7a5315ae3daff0619756988da5311	like_widget

I have added the "Vote: Voted entity" field with "Entity ID" formatter. And the result is blank. If I show the SQL in Views, it is doing this:

SELECT "votingapi_vote"."id" AS "id"
FROM
{votingapi_vote} "votingapi_vote"
WHERE ("votingapi_vote"."type" IN ('updown')) AND ("votingapi_vote"."entity_type" LIKE 'paragraph' ESCAPE '\\')
LIMIT 11 OFFSET 0

No attempt has been made to fetch the entity ID column.

I believe it does work for the entity type of node. The views integration is just broken for other entity types.

🇬🇧United Kingdom Rob230

I do have to still use this module on a live site, so could potentially minimally maintain it (we'll be needing to update to Drupal 10 soon).

🇬🇧United Kingdom Rob230

I don't currently have time to do this, but if I ever add it to a site with Drupal Commerce then it may get done. I'm thinking we can have a submodule that depends on the commerce module.

🇬🇧United Kingdom Rob230

The advice I've seen is not to have the request stack as a required in the constructor because it's only available for HTTP requests, so may not be available in tests. The pattern of having getters and setters which optionally set it on creation if it's available is is how it's done elsewhere in Drupal (Forms, Views).

On the other hand, we are currently using the Request object without checking it exists, and the code isn't going to work without it, so it probably does make sense to inject it, and get the current request at time of use only, not at time of construction.

🇬🇧United Kingdom Rob230

I wondered if it could be a time zone issue since the UK is on daylight savings time (+1 hour), but I don't understand why it wouldn't always use the same time zone.

🇬🇧United Kingdom Rob230

Withdrawing this because it is not an issue with time_field, it was another module.

🇬🇧United Kingdom Rob230

Just casting it to an int would be the easiest solution.

🇬🇧United Kingdom Rob230

I've removed this patch from one of our websites and I'm not seeing the original issue occurring. So this can be closed.

Production build 0.69.0 2024