Netherlands
Account created on 26 November 2012, over 11 years ago
#

Merge Requests

Recent comments

🇳🇱Netherlands Neograph734 Netherlands

Thanks for reporting. In my experience, it was the url method which was unreliable and multiple applications did not load the images.

You could perhaps have a play with the image styles of the base 64 method? It might help.

I am very (very) slowly working on version 4 of the module which uses a plugin based apporoach. I'll keep this request open for improving the image plugin on that version.

🇳🇱Netherlands Neograph734 Netherlands

I have updated the IS and merged it with the content of 🐛 Toolbar causes a Javascript error if the subtrees content changes between page loads Closed: duplicate .

The patch from #2 and #9 does not appear to work. It makes the issue go away, but when a new module is installed, the menu does not update and the new links don't become visible. So the hash might be too generic(?).

The patch from #13 removes the has check alltogether. Without knowing what it is supposed to do, that feels too big of change.

Should we perhaps try hasing the raw menu tree structure or something instead?

🇳🇱Netherlands Neograph734 Netherlands

Thanks @smustgrave, that does indeed look like a duplicate. And I suppose the root cause analyses there also makes more sense.

The recent patch looks like the hotfix from #6, but I am not convinced of that. I think the initial solution direction of 🐛 Faulty Toolbar Subtree Hash Needs work is a lot better.

🇳🇱Netherlands Neograph734 Netherlands

Adding as related issue, so there is a clear cross reference. They look indeed highly similar, even the solution is more or less the same.

I am not sure which one should remain. The other thread has more linked issues, so that might make slightly more sense?

🇳🇱Netherlands Neograph734 Netherlands

Please have a look at the the related issue of the Redirect module ( 🐛 Setting 'Enforce clean and canonical URLs.' breaks CSS aggregation on multilingual Drupal 10.1.x with browser caching enabled RTBC ). It tampers with the newly introduced routes for lazy built css and js files causing issues.

You can't replicate the issue with core and entity print. Only when you add redirect and enable the "enforce clean urls" option you will start to see the issue.

🇳🇱Netherlands Neograph734 Netherlands

Possibly, but that would cause overhead for all other request? Note that this only occurs for the first PDF after clearing the cache.
Or you should keep track of when the cache was last cleared and when each URL was prefetched. I don't think you want to go that route...

I honestly don't think there is anything to fix in this module.

🇳🇱Netherlands Neograph734 Netherlands

From what I have been able to debug, this has nothing to do with a race condition or waiting. It was DOMpdf simply skipping the provided css file because it's content type was wrong (because of a redirect).

It could be that others are facing another issue with similar effect, but I would encourage them to disable the mentioned option in the redirect module first and see if that fixes the pdfs.

🇳🇱Netherlands Neograph734 Netherlands

I am not sure about the RTBC. It was not my intention to have the hotfix committed. Just to have something while we are looking for the cause of the problem.

But I must say that after I discovered the issue from #12 I can do without the hotfix.

🇳🇱Netherlands Neograph734 Netherlands

Thanks for the fast reply! I believe that 10.2 is planned for next week or so? No need to create a new release now then.
I was only asking because I figured that I was still on the dev version.

🇳🇱Netherlands Neograph734 Netherlands

I have found the (my) root cause. The Redirect module's "Enforce clean and canonical URLs." option does not (yet) play nice with the optimized CSS files and adds a redirect. This redirect has the text/html Content-Type with causes DOMpdf to break. There is already a patch in that module's queue: 🐛 Setting 'Enforce clean and canonical URLs.' breaks CSS aggregation on multilingual Drupal 10.1.x with browser caching enabled RTBC .

I think this can be Closed (works as designed)?

🇳🇱Netherlands Neograph734 Netherlands

@fgm thanks for all the work. Is there any chance you could create a tagged release with these changes included?

🇳🇱Netherlands Neograph734 Netherlands

@flocondetoile, I am not sure if you are having the same issue. This issue is specifically about the PDF layout being broken only once after clearing the cache.

From your comment #7 it looks like your PDFs are always broken. Did you follow the instructions in the changelog https://www.drupal.org/node/2888767 ?

🇳🇱Netherlands Neograph734 Netherlands

Upon further investigation I have discovered that DOMpdf does not load the CSS resource because the mimetype of the realtime generated CSS is text/html instead of the expected text/css.

Drupal's CssAssetController does set a content type of text/css. But for the first request this gets cleared somewhere.

Still not sure if this is an issue with Drupal, Symfony or dompdf though... It does not appear to originate from this module though.

🇳🇱Netherlands Neograph734 Netherlands

I have temporarily disabled the CSS optimization by overriding line 132 of entity_print/src/PrintBuilder.php:

$print_engine->addPage($renderer->generateHtml($entities, $render, $use_default_css, FALSE));

This last property determines if the CSS should be optimized. This value moves through the Renderer to the AssetRenderer to Drupal Core's AssetResolver. From there, the asset.css.collection_optimizer service is called to optimize all CSS files. This old service now uses the new CssCollectionOptimizerLazy class.
CssCollectionOptimizerLazy cleary states that it does not write the optimized files to disk, but assumes that they wil be created once the URL is requested.

For all other people watching this issue, are you using DOMpdf? Could it be that DOMpdf simply does not wait long enough for the optimized content to be created or are you using other rendering engines?

Attaching a hotfix with the above change, so that PDFs shoud work again.

🇳🇱Netherlands Neograph734 Netherlands

Thanks ELC, I believe the block is showing again.
Leaving at NR because of #24.

🇳🇱Netherlands Neograph734 Netherlands

Adding related issue with overlapping patch.

🇳🇱Netherlands Neograph734 Netherlands

I am afraid that somewhere in Grevils changes the js has gotten corrupted.

When I patch with the first commit made in this issue my cart block works as expected.
You can use this patch link for composer patches: https://git.drupalcode.org/project/commerce_cart_flyout/-/commit/9cca30d...

Since the last commit from AlexBukach is not in there, you will have to use the patch from 🐛 after upgrading to Commerce Core 8.x-2.36 this module doesn't work RTBC which fixes the same thing. (That is what I did.) You could probably isolate the last commit from this MR in a similar fashion as I isolated the first commit too.

🇳🇱Netherlands Neograph734 Netherlands

Adding in the fork, I am getting an empty div instead of my cart block

I am experiencing the same, although this used to work earlier. Still trying to figure it out.

🇳🇱Netherlands Neograph734 Netherlands

This patch no longer applied for me. It appears that the same changes have now been made in the pull request in 📌 Drupal 10 support Needs work . (But my cart still does not load...)

🇳🇱Netherlands Neograph734 Netherlands

Thanks for taking the time to have a look!

- Omit rendering of empty property lines. I believe the prior setup accomplished as much via conditional output in the template. I just pushed this change into the issue fork.

This makes sense. Thanks.

- I suspect the render() method in the Photo plugin is not necessary, given the render method in VcardPropertyPluginBase.

The render method for the photo plugin is different because it wraps base64 encoded data at 64 characters instead of 75 which is the limit for other vCard properties per https://datatracker.ietf.org/doc/html/rfc6350#section-3.2.

- I wonder if supportsHomeWorkType might be better held in the annotation config / plugin definition...

I suppose I was a bit biased by how Views does this for display/row/style plugins. But it sure makes sense to move it.

- It seems to me that the plugin interface could drop prepareOutputString(), as it seems to be more of just a helper-method in VcardPropertyPluginBase, with nothing outside of VcardPropertyPluginBase depending on it. The render method seems like the one to keep in the interface since it's what the rest of the system hooks up to for output from the plugin. It's possible a plugin could decide to render by other means (or more directly), without following the prepareOutputString() then render() model.

I was definatly thinking of a new major relase, because the compatibility with the old template (and possible overrides people made there). The upgrade path is not impossible to write. It is just a long list of old properties to read and the new format to put them into. That only requires time...

🇳🇱Netherlands Neograph734 Netherlands

It took me quite a while to figure out why my aggregates were not created and I got served all individual files instead.

Apparently this dynamic aggregation does not work in maintenance mode.

🇳🇱Netherlands Neograph734 Netherlands

I ran into the same issue (and came to the same conclusion as larowlan). Here is the change record: https://www.drupal.org/node/3301716

Be aware this this is not limited to webforms, but to every generated pdf (I am using DOMpdf).
It looks like entity_print (or DOMpdf) does not wait long enough for the dynamic aggregates to be created. Bumping to major as this might affect a lot of sites.

🇳🇱Netherlands Neograph734 Netherlands

@chrisolof, I don't know why the commit messages don't show up here, but I have been playing in the issue fork. If you have some time, please give it a try. Adding the NOTE property is now a matter of copying one of the provided classes and making minimal changes.

Be aware that there is no upgrade path yet, so please try the module in a development environment.

🇳🇱Netherlands Neograph734 Netherlands

Oh bummer. I was incorrectly assuming this module was open to complete v4.0 property support.

Don't get me wrong, I am not against it. However, I don't think it would be compatible with the way this module is currently built. There would be endless lists of properties everywhere and people with overridden templates would not be able to see the newly added fields.

I suppose that the better way would be to overhaul the whole field assignment form. Instead of putting all the properties there, use a drop-down to the select the vcard property and then assign fields (to sub-properties if applicable). But then I suppose that all properties would have to be created as annontated plugins with their own form and rendered output. It would be quite the overhaul...

I'll try to have a look the coming days, but cannot promise that I will ever finish it...

🇳🇱Netherlands Neograph734 Netherlands

Committed to -dev. Please let me know if it works and then I will tag a new release.

🇳🇱Netherlands Neograph734 Netherlands

Thanks for making me aware of this and taking a first stab at the patch. I believe it requires a {{ "\n" }} to have a proper line break and then it will pass.

I'll gladly merge the patch then.

🇳🇱Netherlands Neograph734 Netherlands

Hi, thanks for the feedback. However, creating UI fields for all custom properties feels a bit overkill. There is also no support for the SOUND property, to name something.

The UI is designed to work for most use-cases. If you require other properties, I suppose you are better off overriding the module's template file (views-vcards-view-row-vcard.html.twig) in your own theme.
You can either misuse one of the other fields, or use a hook_preprocess_HOOK to add a new variable to the list.

🇳🇱Netherlands Neograph734 Netherlands

Committed. Thanks!

🇳🇱Netherlands Neograph734 Netherlands

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

🇳🇱Netherlands Neograph734 Netherlands

Is this still an issue? I've attempted to fix a similar (the same?) issue in 🐛 DomPdf stops on encountering an unrecognised CSS property and should not Fixed , which is present in the latest release.

The approach there was to no longer treat the DomPDF messages as errors. By suppressing the PrintEngineException entirely as proposed in this patch, I suppose you will run into strange and hard to debug behavior when you cannot write the PDF (permission issues, out of disk space, etc.)

As explained in #3323264-5: DomPdf stops on encountering an unrecognised CSS property and should not , we could still improve on a debugging system.

🇳🇱Netherlands Neograph734 Netherlands

This does appear to be a good and simple alternative for most use cases.

https://www.drupal.org/project/authenticated_frontpage

A very minimal module that let you set up a custom node as the front page for authenticated users.

🇳🇱Netherlands Neograph734 Netherlands

That one was supposed to fail. Setting to NR to validate the claim from #25 and determine if the DB update is needed or not.

🇳🇱Netherlands Neograph734 Netherlands

Made the requested changed from #24.

Tagging 'needs review' to validate the claim from #25 and determine if the DB update is needed or not.

🇳🇱Netherlands Neograph734 Netherlands

@quitone, Thanks for having a look. I've left out the patch because the issue should theoretically be self-healing.

The locale.translation_status key value store has a timestamp and last_checked date. So, when enough time has elapsed the cache should eventually expire and new translations should be downloaded again. Right?
It might be a good idea to validate this though.

If somebody decides they still want an update hook, just calling locale_translation_clear_status() should be enough. Drupal core always clears all translation caches at once, so there is no need to select specific languages.

🇳🇱Netherlands Neograph734 Netherlands

Also, more importantly, I think there is no reason to have separate modules to support that use case: we could just as well have the two plugins in the same module, and spare the extra load for an additional module.

Please also check comment #7. It is split because of the dependency on the media module.

🇳🇱Netherlands Neograph734 Netherlands

@smustgrave IS updated. The essence is more or less the same, but more focus on cleaning up during deletion (instead of fixing it when enabling) as suggested in #9.

🇳🇱Netherlands Neograph734 Netherlands

I was so confinced I had configured git for windows right... :s

🇳🇱Netherlands Neograph734 Netherlands

There is already a lot of cleanups happening in locale_configurable_language_delete(). So it could be as simple as this right?

🇳🇱Netherlands Neograph734 Netherlands

FYI, casting to string is a requirement for assertSame. See the link in failing test of #184.

🇳🇱Netherlands Neograph734 Netherlands

I think this resolves the earlier mentioned issues. So setting Needs review.
Thanks mrshowerman!

🇳🇱Netherlands Neograph734 Netherlands

From the looks of it, it does not even have to be enabled...
It is listed as a dependency in composer, but not in the module .info file. It only downloads it.

So, I am curious what it is and why it is needed (or not).

🇳🇱Netherlands Neograph734 Netherlands

This request already exists.

🇳🇱Netherlands Neograph734 Netherlands

Please be aware that the Drupal permission has a special treatment for user 1. It more or less bypasses all access checks. This affects all modules, not just this one. Please check if the same issue also occurs if you create a new user with the admin role.

🇳🇱Netherlands Neograph734 Netherlands

Confirming this works and it is a much better method than what I used.

🇳🇱Netherlands Neograph734 Netherlands

The field name and transition id are passed to the form builder, so you can recover the transition. I have always used below approach:

  $build_info = $form_state->getBuildInfo();
  [$field_name, $transition_id] = $build_info['args'];

  $state_item = $build_info['callback_object']->getEntity()->get($field_name)->first();
  /** @var \Drupal\state_machine\Plugin\Workflow\WorkflowTransition $transition */
  $transition = $state_item->getWorkflow()->getTransition($transition_id);

But I must say that your method looks much better. It can then be accessed via $form_state->getFormObject()->getTransition()?

🇳🇱Netherlands Neograph734 Netherlands

Hi Andrew,

Please realize that I created this post 4 years ago. I currently have no need for this module anymore and I don't fully remember what I thought back then.

But yes, your steps look complete.

if ($datasource->getEntityTypeId() == $this->entitytypeId) {
I suppose you can skip this check to have the trait compatible with all entities.

And I suppose you can add a new class for general entities, then have the node, and other classes extend that one for backwards compatibility. (Optionally mark them deprecated.)

But this is all the guidance I can currently give. In comment #8 I only wanted to warn you that you should not spend a lot of time not solving the issue.
Thanks for working on this anyway!

🇳🇱Netherlands Neograph734 Netherlands

Although this appears to be removing a lot of duplicate code, it is not solving the issue I initially described.

I was requesting a universal entity bundle processor for every entity type. Something that should be possible with the direction given in the issue summary.

Production build 0.69.0 2024