Live feed

⚡️ Live updates new comments are added automatically.
🇦🇺Australia acbramley

Based on #7 it seems like this can be closed.

Please feel free to reopen if there is still a bug to be fixed here.

🇦🇺Australia acbramley

I tried to reproduce the install/uninstall error in https://www.drupal.org/project/drupal/issues/2743175#comment-16228964 🐛 SqlContentEntityStorageSchema::getDedicatedTableSchema assumes that a fields property definitions matches the columns Needs work to check if this was a duplicate but I'm unable to reproduce the error anymore.

🇦🇺Australia acbramley

Tried to reproduce this on HEAD using a base field on Node:

1. Install standard
2. Add the following to node.post_update.php:

function node_post_update_foo(): void {
  $field_definition = BaseFieldDefinition::create('map')
    ->setLabel(t('Foo'))
    ->setRequired(TRUE)
    ->setDisplayConfigurable('form', FALSE)
    ->setDisplayConfigurable('view', FALSE);
  $definition_update_manager = \Drupal::entityDefinitionUpdateManager();
  $definition_update_manager->installFieldStorageDefinition('foo', 'node', 'node', $field_definition);
}

3. Add the following to Node::baseFieldDefinitions:

    $fields['foo'] = BaseFieldDefinition::create('map')
      ->setLabel(t('Foo'))
      ->setRequired(TRUE)
      ->setDisplayConfigurable('form', FALSE)
      ->setDisplayConfigurable('view', FALSE);

4. Run drush updb -y

No errors, the field is added to the node_field_data table

5. Try the opposite:

function node_post_update_bar(): void {
  $definition_update_manager = \Drupal::entityDefinitionUpdateManager();
  if ($field_definition = $definition_update_manager->getFieldStorageDefinition('foo', 'node')) {
    $definition_update_manager->uninstallFieldStorageDefinition($field_definition);
  }
}

No errors, the field is removed from the node_field_data table.

Has this been fixed elsewhere?

🇨🇦Canada joelpittet Vancouver

Changing to Needs review for the patch. It would be nice to move this to an MR as well for easier code review.

🇦🇺Australia acbramley

Based on our issue category definitions I don't think this should be considered a bug.

🇦🇺Australia nigelcunningham Geelong

My mistake; the issue was elsewhere - views already adds arguments as cache tags, regardless of how they're sourced.

🇦🇺Australia acbramley

It's not clear to me how to reproduce this, but I tried installing from standard profile on HEAD and enabling the settings_tray module. I then used the quick edit contextual link on several different blocks on the homepage and they were always full height.

🇺🇸United States jdleonard Austin, TX, USA

Only lightly tested. The code makes more sense to me this way.

🇦🇺Australia acbramley

Triaged as part of bug smash. It doesn't seem like this is a bug and I'm not actually sure if it'd be possible (or beneficial) to combine the 2.

Element::isVisibleElement is a static function so doing anything with access_callback would be tricky, and it may be intentional that this code doesn't try to process that callback. That would mean isAccessibleElement would just be checking something like ($element['#access'] instanceof AccessResultInterface && $element['#access']->isAllowed()) || ($element['#access'] === TRUE) but then ::doRender needs to do those checks anyway so it can add the cacheable dependency, etc.

With that in mind, I'm not sure if this is something we need to do?

🇳🇿New Zealand RoSk0 Wellington

Tests are a good thing to have, but why replacing dependency injection with a static calls to \Drupal?

🇺🇸United States luke.leber Pennsylvania

I'll have to look a bit closer but the star-rating.js library seems to be a bit susceptible to layout shifting.

Ideally, the markup for an element would be server-side rendered so there aren't any DOM reflows on the client side.

🇨🇴Colombia rvalencia

Propuesta para está publicación

🇺🇸United States dmundra Eugene, OR

Attaching tugboat screenshots from 🐛 Invalid boolean values in recurring_events_create_form.js Active where it is failing and from here where it is passing.

Failing:

Passing:

🇺🇸United States illeace

A little more information about the jquery pathing error:

  • In src/Plugin/Field/FieldFormatter/H5PDefaultFormatter.php line 113, the gets an array of JS files via
    $scripts = $this->assetResolver->getJsAssets($assets, TRUE);
  • Farther down (line 133), the 'data' value gets added to the $jsFilePaths array.
  • For the jquery library, this path is relative -- modules/contrib/h5p/vendor/h5p/h5p-core/js/jquery.js
  • Other path values are absolute, starting with /sites/default/files/js/...
  • This info gets added to the $h5p_integration variable, in line 165
    $h5p_integration['contents'][$content_id_string]['scripts'] = $jsFilePaths;
  • All of this is passed into drupalSettings and eventually used the vendor/h5p/h5p-core/js/h5p.js in the H5P.getHeadTags function to generate this markup:
    <script src="modules/contrib/h5p/vendor/h5p/h5p-core/js/jquery.js"></script>
  • Because the path is still relative, the browser appends that path to the current path (e.g., /node/24/), which results in the jquery loading errors people are noticing.

One way to eliminate the error would be to make sure any script paths added to the $h5p_integration variable are converted to absolute. However, I'm not certain this is the correct solution. A few questions I still have:

  1. This jquery library is already being loaded by Drupal, so this script tag seems redundant (at least for me in this case).
  2. One of the array elements returned by $this->assetResolver->getJsAssets in line 113 is a "drupalSettings" array. This array gets passed all the way to h5p.js and the H5P.getHeadTags function which results in the markup <script src="[object Object]"></script> that generates another of the loading errors. Presumably that item should never get passed to the JS
🇨🇴Colombia solvey

Buenas tardes.

Gracias @rvalencia y @mfernadasilva. Procederemos con la publicación

🇫🇷France pdureau Paris

We are closing alpha4 soon

🇫🇷France pdureau Paris

We are closing alpha4 soon

🇨🇴Colombia rvalencia

Esta es la propuesta para la publicación

🇺🇸United States circuitcipher

I have improved the test code coverage for the Layout builder compatibility.

@ivnish, I used xdebug code coverage to verify that the test cases were hitting all relevant code branches. While I was researching on Drupal code coverage, I saw other modules who have integrated code coverage into their pipelines. Is this something you would want to do? See example here: https://www.drupal.org/project/knowledge/issues/3526796 📌 Add Code Coverage Active

🇮🇳India vinaysamant Kudal, Maharashtra

ok... so is this done? can we close these issues? or u will do it?

🇺🇸United States bradjones1 Digital Nomad Life

The spec says:

Note: Putting a property like "totalPages" in "meta" can be a convenient way to indicate to clients the total number of pages in a collection (as opposed to the "last" link, which simply gives the URI of the last page). However, all "meta" values are implementation-specific, so you can call this member whatever you like ("total", "count", etc.) or not use it at all.

🇫🇷France pdureau Paris

✅ OK, finally, I did the fix about permissions in https://git.drupalcode.org/project/display_builder/-/merge_requests/73

Embedding a display (teaser) in the other (full/default) with the ui_patterns_field's: Render Source formatter. None of them are overridden: I don't see the teaser

Will be moved to a follow-up ticket.

🇨🇭Switzerland berdir Switzerland

This missed the same in comment preprocess, I'm going to try and deprecate that as part of 🐛 [PP-1] Node/comment views-based theme suggestions have no cacheability metadata Postponed

I also updated the change record to clarify that this isn't about using node.view instead. Neither is safe and has caching bugs and should not be used at all.

🇨🇷Costa Rica estebanvalerio.h

The patch is doing its job and provides a solution for this "problem". It works propertly on the project we are using it and the patch code itself is well-structured, security-focused, and properly implemented. It addresses all the stated issues:

- Adds missing permissions for price list items
- Makes "Access the price rules overview page" functional
- Fixes Price List entity admin permissions
- Makes Views respect proper view permissions

The patch follows Drupal best practices and enhances security through proper permission granularity.

🇦🇺Australia acbramley

@alexpott that's exactly what ContentEntityStorageBase::createRevision is documented to do from what I'm reading. Without this fix, reverting a Node to a more recent revision that is not current (i.e clicking Set as current revision) has a really weird outcome and essentially bugs out the revision list (try running the tests without the fix and looking at the HTML output or debugging yourself).

Así quedaría con la corrección

¡Colombia sigue dejando huella en Drupal!
Una empresa colombiana se ha unido como colaboradora de la iniciativa Drupal AI, sumando nuestra voz y talento a un esfuerzo global que está definiendo el futuro de la plataforma.
Cada participación es una oportunidad para innovar, inspirar y mostrar que el talento local tiene mucho que aportar al futuro de Drupal.
Conoce más sobre Seed EM y su trabajo en el ecosistema Drupal
https://www.drupal.org/seed-em
#DrupalColombia #IA

🇨🇦Canada b_sharpe

I'm envisioning something like this, where you would first pick your entity type and field, once saved the entity/field cannot be changed, only deleted, but then the automators become available based on the field type, as many as you want can be added and changed, also allowing the result to provide context to the next in line:

Thoughts?

🇺🇸United States kevinquillen

Unavoidable at the time - open a new issue and should be an easy fix. In the long run we should probably figure out a better way to handle this, even though new numeric series models for OpenAI are not too frequent.

🇮🇹Italy apaderno Brescia, 🇮🇹

Hello Tim,

I am contacting you because Mark ( https://www.drupal.org/u/mark_fullmer ) offered to become co-maintainer for Mime Mail ( https://www.drupal.org/project/mimemail ), a project for which you are project owner and maintainer.

May you post a comment on https://www.drupal.org/project/projectownership/issues/3524942 💬 Offer to co-maintain mimemail module Needs review about accepting or declining the offer? Please do not reply via email; we need a reply on the offer issue.
Without a comment posted on that issue in the next 14 days, Mark will be probably made co-maintainer.

Project moderators will not remove the existing maintainers/co-maintainers; the project owner will not be replaced either. Maintainers cannot change the project owner; co-maintainers/maintainers can only be removed/added by people who have the permission to administer co-maintainers/maintainers.

As last note: This offer is about being co-maintainer, which is different from being a maintainer. A co-maintainer is a person who does not have all the drupal.org permissions on a project. Even though being co-maintainers could mean having just a single permission, we expect a co-maintainer to have the following permissions on the project: Write to VCS, Edit project, Maintain issues, Administer releases.
If there is any reason for not giving all those permissions, please explain that on https://www.drupal.org/project/projectownership/issues/3524942 💬 Offer to co-maintain mimemail module Needs review . We need this to know it was intentional and not a misunderstanding on what the offer required.

Best regards,
Alberto Paderno
-- Drupal.org project moderator
-- Drupal.org site moderator

The status is Postponed because we are waiting for a reply.

Please post a comment after 14 days, if your offer has not been declined. It will show you are still interested in maintaining this project and it will serve as reminder an action is required for this offer.

leymannx Berlin

Hmmm, okay, just noticed now that #10 is just reverting what was done in 💬 The link on the Image tag is redirecting to an undefined page from the node preview screen. Active , so I guess this can't be the fix. Looks like we have to check both. Like this maybe:

let href = event.currentTarget.href;
if (href === undefined) {
  href = event.target.href;
}
window.top.location.href = href;
🇮🇹Italy apaderno Brescia, 🇮🇹

The project link is https://www.drupal.org/project/mimemail .

The only person who has the Administer maintainers permission is tr, who logged in at least once in the past six month. I am going to contact him.

🇨🇦Canada danrod Ottawa

As suggested, I'll merge it to the 2.0.x branch.

🇨🇭Switzerland berdir Switzerland

Yeah, maintaining update tests is a pain, not really motivated to do that. Merged.

🇮🇹Italy apaderno Brescia, 🇮🇹

Thank you for applying!
The project you want to use for this application is not maintained by you. It is not possible to use those projects because, with the workflow we adopted, you are supposed to make commits basing on the reviews done, not (for example) create an issue for the used project and wait the issue fork is merged.

🇮🇹Italy apaderno Brescia, 🇮🇹

Welcome to drupal.org!
This issue queue is not for reporting issues found on contributed Drupal modules or Drupal core; bugs found in a modules/themes must be reported in their issue queue.

🇮🇹Italy apaderno Brescia, 🇮🇹

Usually, after reviewing a project, we allow the developer to opt projects into security advisory coverage. This project is too small for us; it does not contain enough PHP code to really assess your skills as a developer.

Do you have any other project hosted on drupal.org that we could instead review? It needs to have most of the commits (but preferably all the commits) done by you, in at least a branch.

🇮🇹Italy apaderno Brescia, 🇮🇹

Thank you for applying!

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

The important notes are the following.

  • If you have not done it yet, enable GitLab CI for the project, and fix what reported from the phpcs job. This help to fix most of what reviewers would report.
  • For the time this application is open, only your commits are allowed. No other people, including other maintainers/co-maintainers can make commits.
  • The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application.
  • Nobody else will get the permission to opt projects into security advisory policy. If there are other maintainers/co-maintainers who will to get that permission, they need to apply with a different module.
  • We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

To the reviewers

Please read How to review security advisory coverage applications , Application workflow , What to cover in an application review , and Tools to use for reviews .

The important notes are the following.

  • It is preferable to wait for a Code Review Administrator before commenting on newly created applications. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
  • Reviewers should show the output of a CLI tool only once per application. The configuration used for these tools needs to be the same configuration used by GitLab CI, stored in the GitLab Templates repository.
  • It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues .

🇺🇸United States tim bozeman

I used AI to extract the twig template editing feature from Component Library to a sub module, cl_override_mode. While this is a great start, it still needs some work.

🇨🇦Canada b_sharpe

I think this form is actually the wrong spot for these, the field form is for the field config (i.e. field.field.node.article.field_image.yml), however, Automators are their own config entity (i.e. ai_automators.ai_automator.node.article.field_image.default.yml)

There is technically already an admin interface here at /admin/config/ai/ai-automators/ai-automator:

Why not utilize this to handle automators and just link to it from the field config form? Given the idea seems to be to allow multiple automators on a field instance, you could also set this up similar to image styles in which each automator chain from the next using weight to determine order?

🇮🇹Italy apaderno Brescia, 🇮🇹

To make it clear: This is only the first step. I am not still clear who should do the next step.

🇨🇦Canada brunodbo

I opened a merge request with the proposed fix. Also attaching a patch to use in composer.

🇬🇧United Kingdom oily Greater London

oily made their first commit to this issue’s fork.

Automatically closed - issue fixed for 2 weeks with no activity.

🇬🇧United Kingdom oily Greater London

oily made their first commit to this issue’s fork.

🇬🇧United Kingdom nexusnovaz

Moving this to needs review. It has passed (with warnings) on PHPUnit Tests for PHP 8.5.

Automatically closed - issue fixed for 2 weeks with no activity.

🇬🇧United Kingdom Jons

thanks - it's working now

🇳🇿New Zealand atowl

Hi @lucky723,

Glad you have found a solution. Our site just has a link we put on a template with the correct features. eg if you scroll to the bottom of our page and look at the "Privacy settings" link. This is just defined as
<span class=.... eu-cookie-withdraw-tab ....">Privacy settings</span>
Which is picked up by the main JS of the eu cookie complaince.

Happy to have helped you out, and I'll close this issue off now.

🇺🇸United States devanbicher

Here are the steps to setup a site to expose this functionality

  1. Enable mercury editor and inline entity form
  2. Add a new content type
  3. Add a layout paragraphs field
  4. Enable that content type with mercury editor
  5. Create a paragraph
  6. In that paragraph, add a content reference field that references nodes
  7. Have its reference use the content type you created above, save
  8. Edit the new paragraphs' form mode
  9. For the content reference field in this paragraph change the form mode to 'Inline entity form - complex'
  10. Go back to your content type and add this paragraph to the layout paragraphs field
  11. Create a new piece of content (of the type you created in step 3)
  12. You just need to title it, you don't need to add any other content, save
  13. Create another piece of content of the same type
  14. Edit the layout of that new piece of content
  15. Add a paragraph of the type you created in step 6
  16. In the content reference field of this paragraph, select 'add an existing node'
  17. Search for the piece of content you created in step 12 (it should probably be the only one in the list) and select it
  18. After the entity select modal goes away there should now be another button after 'edit' that says 'Layout'
  19. That 'layout' button in the inline entity form is what this patch adds.
Production build 0.71.5 2024