🇦🇺Australia @larowlan

10 years ago

#2225597: contact.module allows you to send emails to users with no email address - producing an error →

This is works as designed for me

Pretty sure this was added recently and is by design, will try and find the issue tomorrow.

Looking ahead I think we should consider also accepting RenderableInterface as well as array. It will allow us to move towards using render element objects without having to call ::toRenderable

Aka 📌 Slowly, very slowly start OOPifying the render system Needs review

But let's fix the regression first and do that elsewhere

Fixing credits

This ended up being really straight forward in the end, much simpler than the previous iterations - because of 🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active

This is ready for reviews

Gave this a manual test against umami, works well.

 php core/scripts/drupal content:export node 3


 [ERROR] The Serialization module is required to export content.


d31d89485d9d:/data/app$ drush en -y serialization
>  [notice] The configuration was successfully updated. 205 configuration objects updated.
>  [notice] Message: The configuration was successfully updated. There are /205/ configuration
> objects updated.
>
 [success] Module serialization has been installed.
d31d89485d9d:/data/app$ php core/scripts/drupal content:export node 3
default:
  revision_uid:
    -
      entity: 1ad0846a-d9f4-4a5b-9a99-ffb9ae6a05c4
  status:
    -
      value: true
  uid:
    -
      entity: 1ad0846a-d9f4-4a5b-9a99-ffb9ae6a05c4
  title:
    -
      value: 'Super easy vegetarian pasta bake'
  created:
    -
      value: 1750135677
  promote:
    -
      value: true
  sticky:
    -
      value: false
  path:
    -
      alias: /recipes/super-easy-vegetarian-pasta-bake
      langcode: en
  content_translation_source:
    -
      value: und
  content_translation_outdated:
    -
      value: false
  field_cooking_time:
    -
      value: 20
  field_difficulty:
    -
      value: easy
  field_ingredients:
    -
      value: '400g wholewheat pasta'
    -
      value: ' 1 onion'
    -
      value: ' 2 garlic cloves'
    -
      value: ' 1 pack vegetarian sausages'
    -
      value: ' 400g chopped tomatoes'
    -
      value: ' 50g sliced sun dried tomatoes'
    -
      value: ' 1 pinch sugar'
    -
      value: ' 3 tbsp red pesto'
    -
      value: ' 50g cheddar cheese'
    -
      value: ' Basil or mixed herbs'
    -
      value: ' 100g mozzarella'
  field_media_image:
    -
      entity: e936d651-a50d-4426-9ab9-fe79d1cac01c
  field_number_of_servings:
    -
      value: 4
  field_preparation_time:
    -
      value: 5
  field_recipe_category:
    -
      entity: 8cb1b744-b885-4885-b0a5-cc8d2fe1498e
  field_recipe_instruction:
    -
      value: |
        <ol>
          <li>In a large pan, boil the pasta in plenty of water until cooked.</li>
          <li>Whilst the pasta is cooking, chop the onion and gently fry it with the garlic in a little oil until soft and the onion looks clear.</li>
          <li>Add the vegetarian sausages. Once browned, remove and chop into chunky bites.</li>
          <li>Pop the sausages back into the pan and add the tomatoes, sugar, pesto and sun dried tomatoes. Season to taste. Simmer until most of the water from the chopped tomatoes has gone.</li>
          <li>Drain the pasta and add to the pan with the sausages and tomatoes. Stir in half of the cheddar and transfer to a shallow dish. Sprinkle with the rest of the cheddar and dot the sliced mozzarella over the top.</li>
          <li>Grill for 10 minutes or until the cheese has melted and started to brown. Serve with basil leaves.</li>
        </ol>
      format: basic_html
  field_summary:
    -
      value: 'A wholesome pasta bake is the ultimate comfort food. This delicious bake is super quick to prepare and an ideal midweek meal for all the family.'
      format: basic_html
  field_tags:
    -
      entity: 2a30ad8a-2e29-4ee0-ad4a-cb7790233ac2
    -
      entity: ec239227-ecb0-47a9-bb5a-af275483ec59
    -
      entity: 8f933857-2858-4092-bf5b-94105148852f
translations:
  es:
    revision_uid:
      -
        entity: 1ad0846a-d9f4-4a5b-9a99-ffb9ae6a05c4
    status:
      -
        value: true
    uid:
      -
        entity: 1ad0846a-d9f4-4a5b-9a99-ffb9ae6a05c4
    title:
      -
        value: 'Pasta vegetariana al horno súper fácil'
    created:
      -
        value: 1750135677
    promote:
      -
        value: true
    sticky:
      -
        value: false
    revision_translation_affected:
      -
        value: true
    path:
      -
        alias: /recipes/pasta-vegetariana-horno-super-facil
        langcode: es
    content_translation_source:
      -
        value: und
    content_translation_outdated:
      -
        value: false
    field_cooking_time:
      -
        value: 20
    field_difficulty:
      -
        value: easy
    field_ingredients:
      -
        value: '400g pasta de trigo integral'
      -
        value: ' 1 cebolla'
      -
        value: ' 2 dientes de ajo'
      -
        value: ' 1 paquete de salchichas vegetarianas'
      -
        value: ' 400g tomates picados'
      -
        value: ' 50g rodajas de tomates secados al sol'
      -
        value: ' 1 pizca de azúcar'
      -
        value: ' 45g pesto rojo'
      -
        value: ' 50g queso cheddar'
      -
        value: ' Albahaca o hierbas mixtas'
      -
        value: ' 100g queso mozzarella'
    field_media_image:
      -
        entity: e936d651-a50d-4426-9ab9-fe79d1cac01c
    field_number_of_servings:
      -
        value: 4
    field_preparation_time:
      -
        value: 5
    field_recipe_category:
      -
        entity: 8cb1b744-b885-4885-b0a5-cc8d2fe1498e
    field_recipe_instruction:
      -
        value: |
          <ol>
            <li>En una sartén grande, hervir la pasta en abundante agua hasta que esté cocida.</li>
            <li>Mientras se cocina la pasta, pica la cebolla y fríela suavemente con el ajo en un poco de aceite hasta que esté suave y la cebolla se vea clara.</li>
            <li>Añadir las salchichas vegetarianas. Una vez dorado, retirar y picar en trozos grandes.</li>
            <li>Pon las salchichas en la sartén y agrega los tomates, el azúcar, el pesto y los tomates secos. Sazone al gusto. Cocine a fuego lento hasta que la mayor parte del agua de los tomates picados se haya ido.</li>
            <li>Escurrir la pasta y agregar a la sartén con las salchichas y los tomates. Agregue la mitad del queso cheddar y transfierelo a un plato poco profundo. Espolvoree con el resto del queso cheddar y salpique la mozzarella en rodajas por encima.</li>
            <li>Asar durante 10 minutos o hasta que el queso se derrita y comience a dorarse. Servir con hojas de albahaca.</li>
          </ol>
        format: basic_html
    field_summary:
      -
        value: 'Una pasta al horno es la comida más fácil y saludable. Este delicioso plato es súper rápido de preparar y una comida ideal entre semana para toda la familia.'
        format: basic_html
    field_tags:
      -
        entity: 2a30ad8a-2e29-4ee0-ad4a-cb7790233ac2
      -
        entity: ec239227-ecb0-47a9-bb5a-af275483ec59
      -
        entity: 8f933857-2858-4092-bf5b-94105148852f
_meta:
  version: '1.0'
  entity_type: node
  uuid: 119ecac6-9dd3-44ea-9ed8-a798a42fcac5
  bundle: recipe
  default_langcode: en
  depends:
    1ad0846a-d9f4-4a5b-9a99-ffb9ae6a05c4: user
    e936d651-a50d-4426-9ab9-fe79d1cac01c: media
    8cb1b744-b885-4885-b0a5-cc8d2fe1498e: taxonomy_term
    2a30ad8a-2e29-4ee0-ad4a-cb7790233ac2: taxonomy_term
    ec239227-ecb0-47a9-bb5a-af275483ec59: taxonomy_term
    8f933857-2858-4092-bf5b-94105148852f: taxonomy_term

Tested it with user 3 and I think we need to look into how password hashing works

content:export user 1
default:
  preferred_langcode:
    -
      value: en
  name:
    -
      value: admin
  mail:
    -
      value: admin@example.com
  timezone:
    -
      value: UTC
  status:
    -
      value: true
  created:
    -
      value: 1751604450
  access:
    -
      value: 1751604506
  login:
    -
      value: 1751604506
  init:
    -
      value: admin@example.com
  roles:
    -
      target_id: administrator
  pass:
    -
      value: $2y$10$XGC/VsNAmnEfIbaIBC7rm.WbYfoeDNgAnV0dx2cwmgx/9V7rE2SNy
      existing: ''
      pre_hashed: false
_meta:
  version: '1.0'
  entity_type: user
  uuid: c71bd884-8881-4d58-b59d-f116082a6117
  default_langcode: en
d31d89485d9d:/data/app$ php core/scripts/drupal content:export user 3
default:
  preferred_langcode:
    -
      value: en
  preferred_admin_langcode:
    -
      value: en
  name:
    -
      value: 'Margaret Hopper'
  mail:
    -
      value: margaret.hopper@example.com
  timezone:
    -
      value: UTC
  status:
    -
      value: true
  created:
    -
      value: 1751604450
  access:
    -
      value: 0
  login:
    -
      value: 0
  roles:
    -
      target_id: editor
  pass:
    -
      value: $2y$10$zF3I5I5J1ILAD/GoXFRxzeqI1sijFx2zjfif1u.QQJXLrj/aQK872
      existing: ''
      pre_hashed: false  # 👈️👈️👈️👈️👈️
_meta:
  version: '1.0'
  entity_type: user
  uuid: 703b9efb-d3d8-4a08-bd63-601f739b1f81
  default_langcode: en

Because pre_hashed is set to FALSE, when the user is imported their password will get re-hashed - see \Drupal\Core\Field\Plugin\Field\FieldType\PasswordItem::preSave and they won't be able to login.

I think we probably want to fix that and add test-coverage.

Other than that, I think this is looking good to go

+1 to this plan

+1 RTBC from me too

4 mins 11 https://git.drupalcode.org/project/experience_builder/-/jobs/5771963 😎

We're green and have PHPUnit running around the 5 min mark.
https://git.drupalcode.org/project/experience_builder/-/jobs/5771862

I think we should probably close this as duplicate of 📌 Refactor \Drupal\link\AttributeXss from SA-CORE-2025-004 Active

In addition we should try to resolve 🐛 Menu link attributes no longer stored in menu_tree since SA-CORE-2025-004 patch Active - we need to make XSS::attributes support HTML5 instead of XHTML.
The former allows underscores in attributes, the later does not.

This is a core issue 🐛 Menu link attributes no longer stored in menu_tree since SA-CORE-2025-004 patch Active

This is a core issue, XSS::filter still uses XHTML filtering where underscores aren't allowed.

larowlan → created an issue.

Postponing on 📌 EntityFormController should load auto save state if it exists Active

,

Added 🐛 Regex for asset library IDs probably not was intended Active and switched tack, because (ab)using that is probably not the best approach

larowlan → created an issue.

First follow-up 🐛 Use of @internal in RenderElementBase property attributes yields phpstan errors Active - getting this in early ++

larowlan → created an issue.

I think it would be simpler to add an entity access control handler that forbid deleting that specific library

I think this is a duplicate of 📌 EntityFormController should load auto save state if it exists Active

@tedbow is working on this

Gating it behind fields with a maxlength is a great idea - nice one

Left a question on the MR

One thing that might be an issue here is that your field-map might be corrupt - which would cause the 'does the taxonomy_forums' field exist on the forum node type to fail.

You could check that with drush if you have it

drush php-eval "echo (\array_key_exists('taxonomy_forums', \Drupal::service('entity_field.manager')->getFieldDefinitions('node', 'forum')) ? 'Field exists in field map' : 'No forums field in field map') . \PHP_EOL;"

Here's the logic of how things should work:

• /forum and /forum/{tid} are built by taking the taxonomy ID and querying the forum_index table to find records
• the forum_index table is written to from node hooks as follows:
  • forum_node_presave runs and sets the forum_id property on the node based on the value in the taxonomy_forums field
  • forum_node_update or forum_node_insert run depending on whether the node is new or not. Both of these start by checking that the node has a field named taxonomy_forums and then if forum_tid is set call the forum index storage server to keep the records in both the forum and forum_index tables up to date

If you have access to a debugger, you may be able to step through this and see where it is failing.

Down to 4 open threads and the new responsiveImage.spec.ts playwright test is failing.

Need some answers on some of the threads

larowlan → created an issue.

larowlan → created an issue.

Agree that 📌 Compatibility with Symfony 7.3 Active got the polyfill in and feels like there's little point in searching for usages where we can use e.g. \array_find in old code.

@mstrelan pointed out we have the 8.4 polyfill in 11.2 but not in 11.1

.

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

Thanks for the updates here folks, the change to remove the extra field.

Cross-linking a core issue about that behavior

@berdir flagged in #22

there are also node_comment hooks that update the search index

They are as follows:

1. \Drupal\node\Hook\NodeHooks1::commentInsert
2. \Drupal\node\Hook\NodeHooks1::commentUpdate
3. \Drupal\node\Hook\NodeHooks1::commentDelete

But comments only reference a node by entity-id, not entity and revision ID, so in that scenario I think we're always going to have the default revision

Setting back to needs review for someone to confirm my thinking on that

Couple of questions on the MR, keeping at RTBC for now

One is an obvious typo that I'll self apply

Credits

Committed to 11.x, nice one

Committed to 11.x - nice one

In \Drupal\Core\Render\Element\Textfield::valueCallback we have return str_replace(["\r", "\n"], '', $input); which unless I'm mistaken is also problematic here because in the case of windows, \r\n becomes \n\n and would result in a different max-length validation to that of the browser - should we be handling that (And any other text-like inputs like telephone, machine name etc) here as well - the issue title doesn't specifically mention text areas.

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

Great work, left some feedback on the MR - but this is pretty close 💪

Not sure why I marked this RTBC 😄

I think we're kind of at the point now where we might need to jump on a call to work out why you're not getting data in the forum/forum_index tables. Will send you an email with some possible times.

Here you go https://www.drupal.org/project/importmaps/releases/1.0.0-alpha2 →

🐛 PHP error on installing Decoupled Layout Builder API Active

Hi @pcambra - I don't think it qualifies for allowed changes for a maintenance minor release → - but thanks for asking. If you feel otherwise, please comment with reasons and/or reach out on slack and we can discuss with release managers.

Will pick up the remaining tasks here tomorrow

Ok was able to trace this down, it was this change in behaviour in 🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active

This was another 'fun adventures in debugging submitting form api forms over JSON' 😭

tl;dr if there are form validation errors, we can't assume what's in the entity field value is anything near valid.

In the case of #23 and #19 it is indeed an array with keys 'date', 'time' and 'object' as seen in DateTime element value callback, and not a string as the entity-api expects.

So the correct behavior is to not only record those form violations, but also make sure the entity in the auto-save state isn't mangled beyond repair. And to only do this for invalid deltas as we don't want to wipe new deltas or valid updates to existing deltas from the auto-save entry.

I've added extra tests and handling for that.

Yes ideally we could do this client-side, and I went down that path but without much joy.

Reverting that behavior change feels like the more robust approach.

Yeah that works for me, the alternative is to add a new method to the auto save manager ::saveSimpleConfig::/getSimpleConfig which is also fine if the new entity-path is too long

Found the cause of #19 - and I think it exists in HEAD, and did even before 🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active because its this in inputBehavioursEntityForm in inputBehaviors.tsx

const validateNewValue = (e: React.ChangeEvent, newValue: any) => {
    // @todo Implement this.
    return { valid: true, errors: null };
};

i.e. we don't do any client-side validation for entity-forms.

In HEAD we don't see this because each POST is loading the default entity.
Before 🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active we didn't see it because we didn't convert until we hit publish.

We see it now because we load the last submitted auto-save entity during POST

The simplest way to trigger this is to add a date timestamp field to the entity form, delete the time and note you get a client-side (browser) error for the required field. Then edit the date field and this triggers a POST to the server, even though other fields on the form are invalid.

I think the fix will be to prevent any submissions while there are HTML5 errors in the form, no different to what any other Drupal form submission does.

@tedbow's response regarding using the hash makes sense - thanks

#21 excellent points - I've:

• Renamed getEntityFields to getFilteredEntityFields as that's a private method so it should convey what it is doing
• Updated openapi.yml
• Added a docblock to ApiLayoutController::get

Actually, I think the new approach in #3529622: Requests to ApiLayoutController::post() that contain the same data as the saved entity will cause a auto-save entry to needlessly be be created makes this redundant. Because conversion happens immediately, so the data stored in the auto save store cannot get out of date w.r.t the source plugin/version.

This is correct, and the test only-change in the MR shows it

Another win for 🐛 Page status changes from "Published" to "Changed" even when no actual changes are made Active 🎉

Linking a known bug with i18n in the contrib module

Committed to 11.x and backported to 11.2.x - thanks folks

Thanks for working on this.

The test added doesn't match the steps to reproduce in the issue summary.

The issue summary mentions that you put in an initial value and it works (which is what the test covers) but then that after you change the value it no longer works - this aspect isn't covered in the test that I can see.

Sounds like we have consensus

Committed to 11.x - congrats folks, this was the oldest open bug in Drupal core, AKA 'the one ⭕️' in #bugsmash parlance

Published change record.

Ok, that all looks correct 🤔

So I wonder why you're not getting records in the forum/forum_index table - that is where this data is read from.

Was this a new site or a site that was migrated from an earlier version of Drupal?

Yes that's correct. If that makes it a stable blocker instead, happy for that change to be made

Committed to 11.x - nice to see this one solved after so many years 💪

Left some comments on the MR, thanks for working on this one

Credits

Committed to 11.x - thanks!

I'll put this to other committers to see if we can get consensus

This looks good to me.

The changes to get RowEntityRenderersTest.php passing reverse test coverage added in 🐛 Cannot use relationship for rendered entity on Views Fixed but I think the tests were asserting the invalid behaviour that has been reported as the bug here.

That said, I'd like to get a review from a sub-system views maintainer because it isn't my strongest skillset. I've set it back to needs review for that and will ping @lendude in slack.

Thanks @jurgenhaas for the extra context

Committed to 11.x and backported to 11.2.x

Nice find folks, really clean fix too

Couple of minor comments on the MR, fine to self-RTBC after those changes

Blocker is in

Committed to 11.x and unpostponed the follow-up - thanks all

Credits

Added 📌 [pp-1] Update use of Range constraint with min 0 to use PositiveOrZero instead Postponed as a follow-up to move other cases of Range w/ min 0 to PositiveOrZero - I agree with @borisson_ - it is more expressive

larowlan → created an issue.

Committed to 11.x - thanks!

Credits

Blocker is in

Blocker is in

📌 [pp-3] Split up and refactor system_requirements() into OOP hooks Active is in

Blocker is in

Committed to 11.x - thanks

@acbramley pointed out that this is already covered by \Drupal\Core\Entity\ContentEntityBase::baseFieldDefinitions

Left a recommendation on the MR for adding an extra guard to ensure the field is of the correct type. Fine to self RTBC after that change.

Committed to 11.x and published the change record

We'll need a new hash in the composer.lock after 🐛 phpstan dev constraints make it difficult for modules to support 11.2 and below Active - fine to self RTBC after that

This broke head on 10.5.x and 10.6.x https://git.drupalcode.org/project/drupal/-/jobs/5716804

Reverting from there and setting to patch to be ported - the class Drupal\file\Upload\FileUploadHandlerInterface was only added in 11.1 (from memory).

Credits