Account created on 6 April 2008, over 16 years ago
#

Merge Requests

More

Recent comments

🇸🇰Slovakia poker10

Thanks for the thoughts @firfin. Week or two ago, I updated the XML Sitemap project description (see https://www.drupal.org/project/xmlsitemap ), so the branches should be described also here. It is true, that the 2.0.x branch is with the D11 support and I do not think we will add it to the previous 8.x-1.x branch. If you feel that this IS should be updated as well, feel free to update it.

I am not aware of any new issues with the 2.0.x branch so far. One of the reasons it was tagged as beta was the fact, that I am not as familiar with the D8+ branches of the module (was previously maintaining only the D7 branch). But if there won't be any new issues, I think that we can tag a stable release soon.

🇸🇰Slovakia poker10

Webform 6.3.x (alpha) is used by 2,159 sites now and that is their decision (because it needs to be installed manually). For example it is not something, which is allowed in most company/goverment enviroments. But Drupal CMS/Forms recipe/PB will make the decision (without acknowledgment) instead of users, which is the difference.

🇸🇰Slovakia poker10

@pameeela We can hide it, but it overlaps with other issues - for example the Forms recipe in Drupal CMS still contains Webform 6.3.0-alpha3, which means that you will end up with a not SA covered module(s) in specific cases, which is not what users expect from a stable product (Drupal CMS 1.0.0). But if we solve this one 🐛 Missing information about the version of the project going to be installed Active , we can probably also list the all the modules which will be installed by a recipe?

🇸🇰Slovakia poker10

I have created one follow-up, which seems more serious than the is-active class: 🐛 Cannot access Project Browser UI for contrib modules listing when on Recommended tab Active

🇸🇰Slovakia poker10

When speaking about hardcoded values on recipes, this is related: 🐛 Security Advisory coverage status is hardcoded on Recipes Active

🇸🇰Slovakia poker10

Yes, It is working with composer create-project. So probably moving just as a documentation issue? Thanks!

🇸🇰Slovakia poker10

Just to note, I have not found an option to turn off the Navigation module feature, that you cannot click on the menu item unless it is a last child.

🇸🇰Slovakia poker10

I have tested the MR with a latest Drupal CMS and the new service seems to block recaptcha when "Use reCAPTCHA globally" is ON and also when it is OFF (so it blocks both https://www.google.com/recaptcha/api.js and https://www.recaptcha.net/recaptcha/api.js). Thanks!

🇸🇰Slovakia poker10

My original report was about views (these are already replaced) and contact form (there is an issue to replace it too). Not sure if it is missing anywhere else. Metatags are configured for "content" with this [node:field_seo_title|node:title], so unless we have any other config pages like the views were, I think this should probably be OK now.

🇸🇰Slovakia poker10

As mentioned on the Slack, I am +1 to update the CNA scope as proposed. This can give us some flexibility in terms of potential security issues discovered after D7 EOL (still 300k+ sites out there) and hopefully also some possibility to have a bit control over the process of CVE publishing and coordination (together with D7ES vendors).

🇸🇰Slovakia poker10

I agree that switching on the items, which Drupal CMS preconfigures, will be the best, if possible. And/or maybe update descriptions on items, which cannot be checked by the module, to clearly state that - so that a user is aware, that the specific item is off, but it has not been automatically checked (so it can be already done).

🇸🇰Slovakia poker10

I imagine a power user would set minimum-stability to their comfort level and users using PB would just click install, not caring about module versions.

@chrisfromredfin - Was there some research done on this topic? From the security point of view, I do not think this will be a general practice. If we look at the Wordpress, you can see, that there is a plugin version displayed if you click on the plugin - so you are aware, what you are going to install. Modules can also have multiple branches, with some new features and/or BC breaks (so you will be affected even if minimum stability will be stable). Not saying that you can jump from security covered release to not covered. This seems to me very important that users are aware, what is going to be installed.

And Drupal CMS set minimum stability to alpha (as of now), so that is even worse: 🐛 Alpha stability flag in composer.json allows project_browser to download any alpha stabiility module Active . And we are talking about non-dev users here.

🇸🇰Slovakia poker10

This is also related to 📌 Update default content to add moderation state Active , because after changes from 📌 Create Basic Content Publishing recipe Active , the homepage is now unpublished. That is causing logged-in users (except admin) to 403 (because the reason in the IS) and logged-out users to redirect to login form.

🇸🇰Slovakia poker10

I think there are two issues here (not sure if I should create a new issue for this, so commenting for now):

1. The proposed solution was to set this as a default page when user click on "Extend". This seems not done
2. When you open the "Recommended" tab, you have "is-active" class on both "Recommended" and "Browse" tabs, which is not ideal

Thanks!

🇸🇰Slovakia poker10

I confirm that when launching the Drupal CMS rc1 zip with ddev in Firefox inkognito, I also see this bug.

🇸🇰Slovakia poker10

@catch There are some other items in the module's checklist. I put to the IS only these which are confusing or not working (sorry if it was not clear from my first post).

🇸🇰Slovakia poker10

Thanks for linking the issue @thejimbirch! I am even more concerned now that I see that it was created and there is no progress towards a fix from August (not blaming anyone).

Given this and reasons mentioned above, I think we should not postpone this issue, instead we should remove the module from the Drupal CMS 1.0.0, unless it will be fixed very soon (or unless other measures will be taken to avoid confusion).

I think it will be much worse, if we keep the module in the current shape included and marketers will start searching and changing the site configuration for no reason, just because of the incorrect reports. Removing the module temporarily seems like a better solution to me, in this situation.

So let's remove the postponed status, if possible. What do you think? Thanks!

🇸🇰Slovakia poker10

Thanks. Created the child issues.

🇸🇰Slovakia poker10

More than two weeks passed without an answer. Moving to the Drupal.org project ownership issue queue.

As explained in the IS, I need permissions to at least update project, write to vcs, maintain issues and administer releases to move forward. Therefore I am asking for the co-maintaintainer role.

🇸🇰Slovakia poker10

at this time, Drupal CMS cannot really be spun up by non-technical people

This also means we should kill the zip file download. Entirely.

Maybe I am wrong and it was not planned as one of the feature of the Drupal CMS (but there was a download link by the Drupal CMS in the initial WF designs: #3453043: Review initial Starshot wireframes ). I think that an easy+non-technical installation is a crucial part of any CMS project. The core is going away from from zip/tarball downloads ( 🌱 [meta] Deprecate tarballs, because they are incompatible with Composer-managed dependencies, Automatic Updates, Project Browser, and release managers' health Active ), which is OK, but Drupal CMS is a different product, for different audience and if we look at other CMS systems, most of them allow "download, copy, install and go" approach (see Wordpress, where you just need to download a ZIP and copy it to the hosting - the same apply to Joomla, Typo3, ...). Anything more complicated could be a barrier to a faster adoption of Drupal CMS, which, as you wrote, is unfortunate. I hope that we will be able to figure out this somehow and change that, because an easy installation is something, which was driving also Drupal 7.

Does this needs to be discussed in a separate issue? Seems like a quite significant change for me. Thanks!

🇸🇰Slovakia poker10

I have mentioned this on Slack, but will post it here too.

Not sure what is the exact status of the MR (I have not reviewed it), but from what I remember, there were some open concerns about keeping the issues IDs, how issue metadata should be migrated ( 🌱 Using GitLab labels for issues on Drupal projects Active ), and similar. @fjgarlin when you will have time to work on this again, it will be great to update the current status (what is done, what decisions are needed and what work is needed).

Given that this will be a one-time migration (for each project), without an option to revert (unlike the GitlabCI migration), I think it would be great, if we can see at least one "demo" project migrated first. We will be able to evaluate and test, if everything went correctly, if all references to issues, comments, etc, are kept and working, how the meta info are migrated, etc. Some feedback could be collected this way before the real migrations via opt-in process will start. Thanks!

🇸🇰Slovakia poker10

Definitely it should be better to set the email to real email address. If we do that, then I think we do not need to disable the contact form email handler. Should we postpone this issue, until 📌 Set the site email to the email address provided by the user Active is fixed, or what is the best approach here? Thanks!

🇸🇰Slovakia poker10

Installed the rc1 without ddev and the message is not there, so I suppose this should be ok :) Thanks.

Regarding the Navigation module and info about the fact that it is still experimental - could we at least mention this on 1.0.0 release notes with a link to the docs page here ( https://www.drupal.org/about/core/policies/core-change-policies/experime... ), so that users are aware, that the module is covered by security advisory policy and should be safe to use? Do we need an issue for this?

Then I think we can close this, as we have other messages covered.

🇸🇰Slovakia poker10

Thanks @ressa! Added a mention about the PHP 8.3 compatibility to the release notes on https://www.drupal.org/project/drupal/releases/7.103 .

🇸🇰Slovakia poker10

Re: for the two unrelated changes, I have already created issues and fixed them (credited @bjaxelsen):
🐛 template_preprocess_emaillog is missing type Active
🐛 Fix translation of strings in EmaillogConfigForm Active

Another thing, which should be considered here is, if we should remove the existing hook - that does not seems ideal from the BC point of view.

🇸🇰Slovakia poker10

Crediting @bjaxelsen from the other issue.

🇸🇰Slovakia poker10

Crediting @bjaxelsen from the other issue.

🇸🇰Slovakia poker10

The patch no longer applies to latest 2.1.x-dev. Please create a MR when updating the patch.

Also this change should be in a separate issue, as it is not related and it is a bug:

diff --git a/emaillog/emaillog.module b/emaillog/emaillog.module
index 9a36f3a..526c116 100644
--- a/emaillog/emaillog.module
+++ b/emaillog/emaillog.module
@@ -137,6 +137,7 @@ function template_preprocess_emaillog(&$variables) {
+  $variables['log']['type'] = $variables['log']['variables']['channel'];

The same applies for this unrelated change:

diff --git a/emaillog/src/Form/EmaillogConfigForm.php b/emaillog/src/Form/EmaillogConfigForm.php
index 6734a43..b4b61c9 100644
--- a/emaillog/src/Form/EmaillogConfigForm.php
+++ b/emaillog/src/Form/EmaillogConfigForm.php
@@ -136,7 +136,7 @@ class EmaillogConfigForm extends ConfigFormBase {
-      '#value' => 'Save Configuration',
+      '#value' => $this->t('Save Configuration'),

Thanks!

🇸🇰Slovakia poker10

Seems like this was already done ( #2844450: Make the initial port of emaillog compatible with Drupal 8.2 ), as there is the admin config form in the latest dev version.

🇸🇰Slovakia poker10

Seems like this was already done ( #2844450: Make the initial port of emaillog compatible with Drupal 8.2 ), as there is errorlog_help and emaillog_help in the latest dev version.

🇸🇰Slovakia poker10

Thanks for reporting this. If someone can create an MR with the fix, I can merge it to the D7 version of the module (until D7 EOL). However I have also seen that translations are used in the maillog submodule as well, so I think we need to check all places, where this needs to be fixed. Thanks.

FWIW, this does not seems to be an issue in the D8+ version of the module.

🇸🇰Slovakia poker10

Thanks for opening the issue and suggestions. Unfortunatelly there are no major changes planned until D7 EOL, which is here soon. I checked and it seems like that at least the issue with translations is not present in the D8+ version of the module. If there are any issues with the recent versions, please open a new issue. Thanks!

🇸🇰Slovakia poker10

Thanks for reporting and working on this. The fix looks correct. I have double-checked the D8+ version of the module and the setting is working.

Committed this to 7.x branch.

🇸🇰Slovakia poker10

Thanks for working on this META. I do not think there is anything actionable here as I have closed the two remaining child issues, see: 📌 [Meta] Porting Watchdog_triggers Closed: won't fix and 📌 [Meta] Drupal 8 port for watchdog_rules Closed: won't fix . Marking as Fixed, because it was done, just without the watchdog_rules and watchdog_triggers modules.

🇸🇰Slovakia poker10

Closing as Won't fix as per parent issue: 📌 [Meta] Porting Watchdog_triggers Closed: won't fix

🇸🇰Slovakia poker10

Thanks for opening this issue. Trigger module was removed from D8 ( https://www.drupal.org/node/2116417 ) and it was abandoned even as a contrib module ( https://www.drupal.org/project/trigger ). Looking at the current status of the Rules module vs the ECA module, I do not think we should still focus to Rules. I think, if anything, the better approach will be to integrate with ECA module (which is also included in the Drupal CMS). Closing this as Won't fix. Feel free to create a new issue for the ECA integration, if there is still demand for such integration. Thanks!

🇸🇰Slovakia poker10

Thank you for creating this issue. Unfortunatelly Drupal 7 will be EOL soon and no new features are expected in the 7.x version of the module. There is an open issue for the 2.1.x branch of the module Custom email subject and message Needs work , so hopefully we can get this in.

🇸🇰Slovakia poker10

Thanks for opening this issue and working on this. Closing as Won't fix as per the parent issue: 📌 [Meta] Drupal 8 port for watchdog_rules Closed: won't fix . In the meantime, the hook_watchdog was already ported to PSR-3 version and the module itself is working.

🇸🇰Slovakia poker10

Thanks for opening this issue. Looking at the current status of the Rules module vs the ECA module, I do not think we should still focus to Rules. I think, if anything, the better approach will be to integrate with ECA module (which is also included in the Drupal CMS). Closing this as Won't fix. Feel free to create a new issue for the ECA integration, if there is still demand for such integration. Thanks!

🇸🇰Slovakia poker10

This seems to be an issue in 2.1.x as well, so moving to this version. I do not expect any changes in the 7.x branch until D7 EOL.

🇸🇰Slovakia poker10

Drupal 10/11 is using PHP_EOL for the mail body line endings, so it is not using \r\n in linux, see: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

That is another reason against using the \r\n for the mail body line endings in linux in Drupal 7.

🇸🇰Slovakia poker10

So, all linux users, must either:

I do not think this is the only one condition which makes this bug present. We have debugged it pretty long and were unable to reproduce the broken emails with the Drupal core itself. So I suppose this is also dependent on mailserver somehow.

The reason why the patch from #45 is not a good idea are explained in #36. This issue was about changing the header line endings. If we have changed also the MAIL_LINE_ENDINGS constant, the mail body line endings would have changed as well, which is something very different (and Drupal 10/11 also does not have this). If there is an issue with mail body line endings, that is probably a separate issue and hopefully can be solved by fine-tuning values of both variables without the need of any custom core changes.

Re:

2- Apply the patch, because in the file "/includes/mail.inc" there is this line (line 10):

I do not think this is correct/needed anymore. The MAIL_LINE_ENDINGS constant is defined on the top of the mail.inc file. Then, when it is used in the mail() function, there is a possibility to override it via mail_line_endings variable:

DefaultMailSystem::mail():

    $line_endings = variable_get('mail_line_endings', MAIL_LINE_ENDINGS);
    // Prepare mail commands.
    $mail_subject = mime_header_encode($message['subject']);
    // Note: e-mail uses CRLF for line-endings. PHP's API requires LF
    // on Unix and CRLF on Windows. Drupal automatically guesses the
    // line-ending format appropriate for your system. If you need to
    // override this, adjust $conf['mail_line_endings'] in settings.php.
    $mail_body = preg_replace('@\r?\n@', $line_endings, $message['body']);
    // For headers, PHP's API suggests that we use CRLF normally,
    // but some MTAs incorrectly replace LF with CRLF. See #234403.
    $headers_line_endings = variable_get('mail_headers_line_endings', "\n");

So everything from in the mail body should be changed to specified newlines by this line:

$mail_body = preg_replace('@\r?\n@', $line_endings, $message['body']);

The precedence is by the variable, not by the constant. If this code is not working as expected, that is a separate issue.

Can you please doublecheck again, why do you need to change the constant at all? If you are using another mailer and the seding is not done by core DefaultMailSystem::mail(), then it is indeed a problem of the contrib module.

Thanks!

🇸🇰Slovakia poker10

Recaptcha has an option use_globally and then the https://www.recaptcha.net/recaptcha/api.js is used instead of https://www.google.com/recaptcha/api.js. Could we also add the second script?

🇸🇰Slovakia poker10

So just to confirm, if we remove that folder in 🐛 Remove .ddev directory from composer create-project Active , then in the Drupal CMS 1.0.0 zip file the warning will not be present? If so, I think then it is OK. Thanks!

🇸🇰Slovakia poker10

Thanks for creating the issues!

But I don't think Drupal CMS can/should remove this warning since it is something the user should be aware of? If it were to appear in the (future) browser-based trial experience, that would be a problem. But for local dev, I think it's OK?

Agree, but I saw the warning when installed the RC1 from zip archive. Is that still considered as a dev version?

🇸🇰Slovakia poker10

There is also another simple use-case how to run into this.

1. Install Drupal standard profile.
2. Enable translation of terms for the Tags vocabulary.
3. Go to permisions form and add translate tags taxonomy_term to at least one role
4. Go back to the vocabulary and disable translation of terms in the Tags vocabulary.
5. Go back to the permissions form and try to save it without changes.

You will get:

RuntimeException: Adding non-existent permissions to a role is not allowed. The incorrect permissions are "translate tags taxonomy_term". in Drupal\user\Entity\Role->calculateDependencies() (line 210 of core/modules/user/src/Entity/Role.php).

Drupal\Core\Config\Entity\ConfigEntityBase->preSave(Object) (Line: 182)
Drupal\user\Entity\Role->preSave(Object) (Line: 528)
Drupal\Core\Entity\EntityStorageBase->doPreSave(Object) (Line: 483)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 257)
Drupal\Core\Config\Entity\ConfigEntityStorage->save(Object) (Line: 354)
Drupal\Core\Entity\EntityBase->save() (Line: 614)
Drupal\Core\Config\Entity\ConfigEntityBase->save() (Line: 1010)
user_role_grant_permissions('content_editor', Array) (Line: 984)
user_role_change_permissions('content_editor', Array) (Line: 257)
Drupal\user\Form\UserPermissionsForm->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 129)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 67)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 597)
Drupal\Core\Form\FormBuilder->processForm('user_admin_permissions', Array, Object) (Line: 326)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

This seems like a very common use-case which can happen on sites and should not result in error.

🇸🇰Slovakia poker10

Re

Going to a 403 seems worse than going to a stub page, maybe it should be published with the stub content?

This part was discussed also in 📌 Default content for privacy requirements Active . I agree that from the UX point, it might me better than 403, but if it will be published, it will be indexed by bots / search engines, which does not seems good, given the content it contains. And we know, that not all sites owners will immediately change the default content - sometimes it will be there for a long time, unfortunately.

🇸🇰Slovakia poker10

There was a discussion about possible outcomes when using minimum-stability together with prefer-stable: #3179061: Further discuss use of "prefer-stable" and "minimum-stability" in core . Not sure everything applies also to Drupal CMS structure, but I think that under some circumstances, using minimum-stability: alpha with prefer-stable: true could have caused a downgrade.

🇸🇰Slovakia poker10

I still do not understand, if there was openstreetmap.de in the prototype (which is for sure a better alternative from the privacy point of view for EU users), what was the reason to switch it to the fastly - is there any explanation? And why it is a problem switching it back?

Asking the user for their consent to deliver external content (that's for videos, fonts, maps, etc.) is compliant with known legislations (including GDPR, CCPA, and others) and not only relevant in the EU but an emerging requirement in more territories around the globe.

In general this is true, but the requirements to companies in EU / non-EU differs, so it is more convenient for users in EU to use EU based service. Unless there are any benefits, or huge amount of work needed for the switch, I personally do not see a reason to won't fix this. But maybe I am missing something? Thanks!

🇸🇰Slovakia poker10

From the Slack discussion:

The projects on d.o. have this text: "Stable releases for this project are covered..."
The projects in PB have this text: "Module is covered..."

So one possible way how to at least lower the impact here is to change the text to match the Drupal Security Team policy.

🇸🇰Slovakia poker10

From the security point of view, there is a policy, that anything non-stable is not covered. If project browser know what is going to be installed (which I suppose it should know), then I think this can be solved.

Other than that, I think that because of the fact that Drupal CMS includes also alpha versions, the composer.json "minimum-stability": "alpha" is hardcoded, so no, sites may not be aware what will happen, which is not good. For sites using project browser outside of Drupal CMS, it could be/is different.

Or am I missing something?

🇸🇰Slovakia poker10

Re #24:

Are facets enough of a common use case where >=80% of sites would want them?

Personally, I am not convinced about this. I see facets as a more complex feature for commerce and bigger sites, but maybe there is something which can be beneficial also for smaller sites?

🇸🇰Slovakia poker10

I think that @saschaeggi mentioned the Trash icon is already fixed in Gin theme in: 🐛 Add navigation icon for Trash Active

This is probably also somewhat related to: 🐛 Status warning: Toolbar and Navigation modules are both installed Active .

🇸🇰Slovakia poker10

I think the more privacy, the better. Yes, there should be a protection until you add consent, but after that, your data will be stored on US servers and it may not have the privacy protection level as it is required on EU servers. Also I think the privacy policy needs to be a bit different in case you are providing data to companies outside EU (if I am not mistaken).

https://techgdpr.com/blog/server-location-gdpr/

Given this was already one in the prototype, I think it is a good idea for EU users, even that we are not speaking about some very rich data (mostly IP addresses, ...).

Production build 0.71.5 2024