I added some styles and icons.
Also included the license for the icons as this must be provided with the icons.
Before we merge this we should finish the maintenance page issue and also check the styles there.
guido_s → changed the visibility of the branch 3544442-404-403-status-codes to hidden.
Sorry, just noticed there is another issue for this created 6 years ago and still not fixed.
This seems to need more work.
We need to resolve Ihors comments and it would be nice to have a solution that not only works with cloudflare turnstyle but with all kinds of captchas or at least the commonly used ones like recaptcha, hcaptcha, or what ever else is used in many sites.
Thanks for also testing this. I rebase the branch and added it to the merge train.
Thanks everybody!
Thanks for the hint Ihor, I totally missed that when I updated the configs.
Fixed that.
So I think we can merge this.
I also just tested it with the viewsreference module and also with the views_field_formatter module and enabled ajax in the view.
Without the new code I can reproduce the ajax error in the console and I can confirm the code fixes it and the view was working fine after clicking the consent button.
I also added the if clause that Ihor also noticed it was missing.
So I think we can merge this now.
I added some more schemas and changed existing schemas.
I also changed the config for installation on the embed iframe module.
Please check again.
Thanks, I tested it again and now the content is properly removed and loaded on consent.
Thanks for checking the code Ihor. As you also approved it, I merged it.
Tested in Drupal 11 and I'm not receiving any schema error messages anymore when placing the block.
Therefor merged it after rebasing.
Tested it in Drupal 11, resolved phpcs issues and merged it.
Ok, let's merge this and see if the pipelines still have any issues outside the fork.
I still don't know why the stylelint pipeline isn't working. The same is working in another module.
I managed to fix phpcs pipeline but this one is still not working. Tried a few different approaches now.
Should we try to fix it or should we merge it and see if it is just an issue in the fork?
@igor mashevskyi and me discussed that we will implement the newly introduced enums in a later D12 release of the module.
But we should still add the link to the download page where the library can be downloaded.
Looks good, everything green in the pipelines and code looks fine.
I'll merge it now.
Please add the mentioned config files. Especially the phpcs.xml with Drupal and DrupalPractice Standards for Drupal 11.
You only added the gitlab-ci.yml. I also want to have other needed config files in there. Especially the phpcs.xml declaring the used coding standards.
Even with the patch from this issue I agree with @eelkeblok that it wouldn't require any db updates.
Also asked the AI:
no update hook needed.
Why:
* The patch only switches to using the parent base field definitions and replaces
t()
calls withTranslatableMarkup
+ some label/description text tweaks.
* It does not add or remove any stored fields, indexes, or change any field storage settings.
* Theid
,uuid
, and bundletype
fields now come from the parent, but their storage characteristics remain the same (integer unsigned id, UUID readonly, bundle reference tovote_type
). The patch just overrides their labels/descriptions.What to do after applying:
* Just rebuild caches (
drush cr
). There’s nohook_update_N()
or post-update required because no schema changes are introduced.
I just upgraded to 4.0.x-dev Version on a platform where I'm using this module with the Rate module with thumbsup rating and stars in entities (nodes and commerce products), views and webforms and had no issues so far without any database updates.
Upgrading drupal/votingapi (3.0.0-beta5 => dev-4.0.x 010514d)
You are absolutely right. Those are some implications I didn't think of. Guess we need a way to distinguish this in the monitoring service then somehow to not mess up the downtime statistics and causing alerts on planned maintenance.
But as this won't affect the module code then, I'll close this issue.
Tested it without any icon set, and with different paths with and without stream wrapper public:// and works fine now.
Also changed the field description as it was not completely correct.
I suggest an approach where we set the status code directly in the controller instead of altering it afterwards with an event subscriber.
Opened a merge request for this too. Please have a look.
It would be great if the module would support using the key → module which seems to be kind of the new standard. There you can choose if you want to save the key in the configuration, an environment variable or in a file.
The branch naming of this fork is not good here. There shouldn't be a branch named 1.0.0-beta1 for an issue. The other branch name (3544410-add_403_and_404) would be preferred. But the content of the branch is good and both install and uninstall hooks work fine and values for the error pages are set and removed as intended.
So I'm gonna merge this, but pay attention on future branch names please.
guido_s → changed the visibility of the branch 3544410-add_403_and_404 to hidden.
I just tested in Drupal 11.2.3 and can confirm there is a placeholder now on both nodes and blocks, but I also noticed the content of the field isn't blocked now. The placeholder just covers the content of the field.
So this needs more work.
<div id="block-olivero-testblockfortestingfieldblockinginblocks" class="block block-block-content block-block-contentbfd273df-2330-489e-be5d-183a295bc5e6">
<h2 class="block__title">Test Block for testing field blocking in blocks</h2>
<div class="block__content">
<div class="cookies-fallback--fields--wrap cookies-fallback--wrap disabled"><div class="cookies-addons-fields-placeholder text-content clearfix field field--name-body field--type-text-with-summary field--label-hidden field__item cookies-fallback--element" data-cookies-service="fields" data-service-name="Fields" data-field-id="block_content-1-body" data-view-mode="full" id="block_content-1-body-content">Test body of blocked field</div><div class="cookies-fallback cookies-fallback--fields cookies-fallback--fields--overlay"><div class="cookies-fallback--text">This content is blocked because field cookies have not been accepted.</div><button class="cookies-fallback--fields--btn cookies-fallback--btn">Accept field cookies</button><a href="#cookiesjsrAccept" class="cookies-fallback--link">Accept All Cookies</a></div></div>
</div>
</div>
I also just tested it in a fresh Drupal 11.2.3 site and can confirm it is still working as expected and placeholders are properly replaced with the actual content.
As Igor also approved it, I'll merge it.
Content can't be kept as this what needs to be blocked.
It must be done somewhat like in cookies_addons_views.
Thought I had seen something like this in the code when I checked @_shy s code. But as mentioned didn't have the time to check the rendered output myself.
@_shy said it would be working fine. If not that needs to be fixed. Everything needs a proper placeholder div which is replaced after cookie consent for the specific category.
That should be changed in the whole module instead of just the cookies_addons_fields module.
Maybe the function changed in the past in the cookies module? Not sure, but you are correct. serviceId is the only expected parameter.
That won't work here. It might be possible for some field types, but fields can hold complete views, load libraries and so on. Nothing that could be easily solved by storing everything in a data attribute.
We should add attributes to the properties in the yml file and only create new attributes if this is not passed on the include. Like:
attributes:
type: Drupal\Core\Template\Attribute
title: Attributes
Additionally I'd prefer if we wouldn't need the items to be strings and could pass render arrays too. I don't want to convert or render everything before.
Thanks for testing it again!
I merged it now and we can create a new feature release and update the modules description.
I added some validation for the route and some access checks to make sure users can only get fields and view modes they have access for to not leak any data. Didn't manage yet to do a test in a test site though. Feel free to check again. I will merge this after it has been properly tested in a real drupal environment.
Checked the merge request and merged it.
Thanks!
Checked and merged!
Thanks!
Thanks, that would be great!
I also checked the project for php compatibility with phpcs
phpcs --extensions=php,module,install,inc,theme,engine --standard=PHPCompatibility --runtime-set testVersion 8.4 cookies_addons/
I checked for all versions from 7.4 to 8.4 and there were no errors or warnings.
So if there still are any issues with php compatibility it will be a runtime error due to wrong parameters, as phpcs can't detect those.
I'll merge it and add a new tagged release.
guido_s → made their first commit to this issue’s fork.
Found the cause of the issue now.
Some email bodies where set as ['#type'] = 'processed_text', but had no ['#format'] assigned.
So the plain_text format was used by default which had the filter 'Convert URLs into Links' active with a 'Maximum Link text length' of 72 characters.
I fixed this now by creating a new EmailAdjuster and a text format for emails, which will be set if there was no format assigned for processed_text.
Like this:
/**
* {@inheritdoc}
*/
public function build(EmailInterface $email) {
$enabled = $this->configuration['enabled'] ?? FALSE;
if (!$enabled) {
return;
}
$legacy_message = $email->getParam('legacy_message');
if (isset($legacy_message)) {
$bodies = $email->getBody();
foreach ($bodies as &$body) {
if (isset($body['#type']) && $body['#type'] === 'processed_text' && !isset($body['#format'])) {
// Check if 'html_email' text format exists before using it.
if ($this->textFormatExists('html_email')) {
$body['#format'] = 'html_email';
}
else {
// Fall back to a default format if 'html_email' doesn't exist.
$body['#format'] = 'plain_text';
}
}
}
$email->setBody($bodies);
}
}
We could add this feature for all types of sliders.
Would be nice to have.
I merged it. Looks fine. Also thanks for replacing deprecated functions.
@_shy can you add a new release?