NH, USA
Account created on 25 January 2007, over 17 years ago
#

Merge Requests

More

Recent comments

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

DamienMcKenna β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

There are some test failures that need to be fixed.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thank you for working on the merge request. It needs to be updated to match the official format: https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... β†’

Thank you.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Once we have the D11 fixes in for both the 8.x-2.x and 3.0.x branches I'll tag both releases.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Once we have the D11 fixes in for both the 8.x-2.x and 3.0.x branches I'll tag both releases.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Committed. Thank you.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thank you for spotting that option, that's a lot better than it was.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Looking at the merge request - aren't the changes to SchemaNameBase.php breaking compatibility with Metatag 8.x-1.x? Those look like changes necessary for Metatag 2?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Committed. Thank you.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

This looks good, thank you.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Test coverage.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

This was fixed in #3308271: RestUIForm should use ModuleHandlerInterface β†’ , so if you update to the dev version you'll get the fix.

Maintainers: please tag a new release of the module so we can have this fix available. Thank you.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Next step: Test coverage.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thanks Klausi.

Put another way - this is a minor security hardening issue for some rare scenarios, rather than a something a non-admin would be able to exploit.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Are these API changes in the 8.x-1.x or 2.x branch? That will make a difference on how we decide to handle this. Thank you.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Please open a new issue for the problem you're running into, it's a different problem to what this issue is focused on. Thank you.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Does it work if you do [node:field_taxonomy_name:entity:parent:entity:field_something]?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Try looking at the Token help info at the Taxonomy structure, those tokens will show what is available from within the [node:field_taxonomy_name:entity] structure, you should be able to stack the tokens from that point, so it's probably something like [node:field_taxonomy_name:entity:parent:name] to get the name of the parent term on the term referenced on the field_taxonomy_name field on the node.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Did you report a bug on the Plugin Pack module?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thank you all for working on the fix and reviewing it.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thank you for putting that together.

Please create the merge request from your issue fork and we can see how it works in the tests.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Hah! I had the wrong entity token in the "entity: set field" action, so it wasn't actually saving the message entity.

Nothing to see here :)

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

The missing step was to add a new action that saves the entity, either the "Entity: set field" action and then set the "Save entity" option to "Yes", or add the "Entity: save" action.

Thanks to jurgenhaas for the help working this out.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Maybe the "Login of a user" event should have an option on whether it logs logins that use Drush?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thanks for spotting that. I opened πŸ“Œ Document that {{ page.attached }} must be in html.html.twig Active to add it to the documentation.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Here's what I'm doing so far.

* I have a message configured with a field_user that's a reference to a user.
* The message uses [message:field_user] etc to loads details about the user on the message.
* Added an ECA model with the event set to user login.
* The model then has an action of "entity: create new", the entity type is the "message: user logs out" item noted above, and owner ID is set to "[current-user:uid]".

This doesn't seem to work.

Questions:
* What should the "Name of token" be set to?
* How do I pass other data to the message, e.g. for field_user? I had been assigning them in custom code, but I'd prefer to manage this logic in ECA rather than custom code, for visibility sake.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

1. It needs to be rerolled for the 2.0.x branch.
2. The UI needs to be improved so it has an initial checkbox that says "Show all meta tags", if that isn't checked then it shows all of the individual tags that are available.
3. Not sure on loading just the raw values with the tokens rather than showing the processed values - you can already see the tokens on the admin page (admin/config/search/metatag) for each entity type and bundle, so I don't see much benefit in that.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Does this break on older versions of PHP?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

a) Product = a course at a college, Product Variant = a specific instance of that course.
b) Whatever scenario this person is using it for: πŸ’¬ Product Variation meta tags not working Active

I'm not saying every scenario needs it, but it should be possible.

Is there a hook or event that allows the entity's definition to be modified? I wonder if this could be done as a separate contrib module?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

I think the root problem is that Commerce product variants don't have their own "canonical" route.

If you look at the Product entity file it has a "canonical" link specification in the entity's definition:

 *   links = {
 *     "canonical" = "/product/{commerce_product}",

If you look at the ProductVariant entity file it doesn't have the "canonical" item in the links definition.

Metatag requires that the current page points to a specific route, especially in order to access the tokens that belong with that entity. This is how it works for nodes, users, terms, etc. Because product variants don't have their own route all of this entity logic doesn't work as expected.

I did open a feature request for Commerce to add this route ( ✨ Add a canonical URL for the ProductVariant entity Active ), we'll see what becomes of it.

The alternative approach is to use the [commerce:current_variation] tokens, as defined in commerce_product.tokens.inc, and add them on the Product entity's meta tag defaults. You might also take a look at the Token page of the Help page, it will show you the entire token tree and it might help track down what you need.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

"Unpublished access"? I think the word "token" won't be clear for non-technical users and shouldn't be used.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

For the DeprecationHelper change, IIRC the short term fix should be to use phpstan's commands to ignore that line, and then we can open a D12 compatibility issue to fix this later on once we're no longer supporting older releases.

I think we need to dig into the D7 code to see where $element_groups came from.

Thank you for making the other changes.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

I renamed the "user_updated" message to "user_update", reran the migration and everything worked. Not sure why this one migration didn't save correctly, but it's an odd one.

Going to close this and chalk it up to gremlins.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Can you please post your d7_message yml file so we can review it?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thanks for the patch, hopefully someone will review it soon (I recreated my message templates so didn't use this).

As a reminder, please set the "assigned" field to "Unassigned" when you're finished.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Great work, everyone! I really appreciate you all for working through this.

A few minor things need some tweaks.

  • While I appreciate that DeprecationHelper exists, I think we should just do the old method_exists() check for now, for compatibility with older core releases.
  • getID3 should be added as a test dependency so that the @phpstan-ignore class.notFound bits aren't needed for getID3().
  • The @phpstan-ignore variable.undefined, empty.variable bits should be redone to avoid needing the phpstan-ignore lines.
  • Why were the "Need more information" titles removed?
  • Code that is commented out should be wrapped with @code / @endcode.
πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

You opt into the security advisory coverage by updating the project and setting the "Security advisory coverage" field to "Opt into"; the project still shows it hasn't been opted in yet.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

The "token.tree" route comes from the Token module. Something must be interfering with that route, which causes it to not work. Either which way, this is not a problem from Metatag, so I'm passing it to the Token issue queue, maybe someone here will have an idea what to look at.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Ok, thanks for letting us know.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

DamienMcKenna β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

The $form['field_metatags'] variable will always depend upon how it's configured on the site, it should be $form['$metatag_fieldname'].

This is a difficult issue to accomplish, because Drupal's form API is so complex and because there can be so many different possibilities on how the meta tags are configured, that's why there hasn't been a solid implementation of this functionality yet.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

FYI I suspect other plugins might also be affected by this.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Moving to the CKEditor 5 Plugin Pack module.

The patch needs to be rerolled.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Refocusing this on deprecating this module.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

I suspect the problem is that the "Limit allowed HTML tags and correct faulty HTML" filter removes the HTML structure that the plugin creates.

I think this will be another scenario where core's "Limit allowed HTML tags and correct faulty HTML" filter always removes the "style" attribute, due to its security implications. The Extended HTML Filter β†’ module aims to replace core's filter with one that allows the "style" attribute, but it has problems, see ✨ Match the filter_html <> ckeditor5 integration in Drupal core Needs work for details.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Skipping this in favor of the Plugin Pack module.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

I think this should be rewritten to use the pagebreak code provided by the CKEditor 5 Plugin Pack module β†’ .

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

How about just adding attributes for the three variables? IMHO that would be a better solution.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Are you able to load the tokens on other admin pages?

Have you tried Pathauto? It uses tokens to generate consistent URLs for different types of content, so might be able to tell if the problem is limited to Metatag.

Do any of the other modules you've installed (besides Token and Token OR) add any tokens to the site? ie. do they have a MODULENAME.tokens.inc file?

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Thank you for putting this together.

I don't think this would work super well:

  • It assumes the Metatag field is called "field_metatags".
  • It only works when the form is initially loaded.

I think the correct approach would be either an AJAX callback of some sort, possibly as a response to a button being clicked, or to do it all in JS on the page.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

FYI I formatted the timestamp like this:

/**
 * Implements hook_preprocess_field() for "field_duration".
 */
function MYMODULE_preprocess_field__field_duration(array &$variables) {
  // Reformat the duration as a time string.
  foreach ($variables['items'] as $key => $item) {
    $duration = trim($item['content']['#markup']);
    $minutes = floor($duration / 60);
    $seconds = $duration - ($minutes * 60);
    $variables['items'][$key]['content']['#markup'] = $minutes . ':' . $seconds;
  }
}
πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Minor mistake on the 10.2 patch.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

I realized that it needed to check the parent plugin for possible attributes if the known one wasn't found.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

A bare version of #4 for loading via composer.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

It might be worth spinning this off as a separate contrib module that replaces core's standard VideoFile plugin with its own and then adds all of the getID3 attributes.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

This uses getID3 to get the duration in seconds, which are rounded to the nearest second.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

Rerolled for 10.2, for anyone who wants to test it.

πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

This adds some of the basic structure necessary, now to work out how to actually have the video's duration as an available value.

Production build 0.69.0 2024