Account created on 6 April 2008, about 17 years ago
#

Merge Requests

More

Recent comments

🇸🇰Slovakia poker10

Merged to 2.x, thanks everyone!

🇸🇰Slovakia poker10

This looks good, thanks. Drupal 10 requires minimum PHP 8.1, so I think that it is safe to add the nullable type declarations.

I reviewed the changes and have not found any other places where the deprecation message can be emitted.

🇸🇰Slovakia poker10

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

🇸🇰Slovakia poker10

@ivnish Not sure if there is an issue for contribs yet (have not found it), but feel free to create one to discuss it. Thanks.

🇸🇰Slovakia poker10

Message looks good and credits are cleared. +1 from me. Thanks!

🇸🇰Slovakia poker10

Seems like the 2.0.x is now used by approx. 10k sites. I do not see any new issues or bugs regarding the D11 compatibility, so I will take a look this week a tag a stable release.

🇸🇰Slovakia poker10

Thanks for reporting this. This is a default behavior of the module if base URL cannot be detected. Site URL needs to be set manually here: /admin/config/search/xmlsitemap/settings in Advanced settings -> Default base URL, or use another method to set it.

See: https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib...

🇸🇰Slovakia poker10

Thanks for working on this. 8.x-1.x branch will not support Drupal 11. There is a 2.0.0-beta1 release, please test that release.

🇸🇰Slovakia poker10

Re #95:
I think that a major issue is that there is no equivalent way (or at least an easy way working generally for all use-cases) to replace the functionality it provided. I think that core can manage it, but for contribs and custom themes/code, it will add a lot of work. If you look at the related issue ( 📌 Remove use of deprecated "spaceless" filter in core templates Active ), not all changes were just removing the filter and adding some whitespace controls. And lot of these changes needs to be tested manually, because it has a potential to disrupt layouts and so. So I see a benefit of providing our replacement until we decide to deprecate and remove it entirely (as proposed in IS), to help the contrib world with the transition (though I am not sure when Twig 4 is planned).

🇸🇰Slovakia poker10

2. Navigation suggests to uninstall toolbar module when navigation is used.

I think Gin already supressed the message in: 🐛 Temporarily suppress toolbar & navigation warning Needs review . It was a workaround, because as per comment #7 from 🐛 Status warning: Toolbar and Navigation modules are both installed Active , it was not possible to uninstall toolbar module yet (not sure if that is still true these days).

🇸🇰Slovakia poker10

However, there was some pushback (#44_) on showing warnings on a standard install, so we need to be sure this is considered.

Yes, I was thinking more about adjusting the existing error messages, which are displayed when .htaccess files are missing, so that these have a different wording when the auto_create_htaccess config is set to FALSE, or hide these messages entirely. I agree that adding a new requirements check with another warning or so is not ideal.

🇸🇰Slovakia poker10

When doing this, do we want to update also this part, to include general committers?

The roles of Project Lead, product manager, framework manager, and release manager have commit access and are collectively referred to as "core committers" (or "branch maintainers" for a specific major Drupal version).

It seems to me like a duplicate of the "Leadership Team: Core Committers" part, but given the roles are explicitly mentioned here, then it is probably not ideal to keep it as is.

🇸🇰Slovakia poker10

Thanks for working on this. Should this be postponed on 📌 "Promoted to front page" for new content types should default to Un-Checked Needs work ? Because the default value for new content types is still promote=1. If this patch is applied and a new content type is created, it is set to promote automatically, but the field is hidden in form display, so it cannot be unchecked easily for content editors, unless brought back. I am not sure this is ideal.

🇸🇰Slovakia poker10

There is still one @todo in the MR - do we need to discuss that? I do not see it mentioned anywhere in this issue. Thanks!

🇸🇰Slovakia poker10

Sorry for asking, maybe it is obvious, but when we added webp format and possible conversion to core and image styles ( 📌 Add webp image conversion to core's install profile's image style Fixed and 📌 All core install profile image styles should include webp conversion Active ), we have not added any fallback. Was webp available in all PHP installations at that time? If not, are we sure that now when avif will not be available, webp will be?

🇸🇰Slovakia poker10

I think that this probably does not need tests, as it is a simple typo from a previous issue (see this commit). Checking by https://www.drupal.org/about/core/policies/core-change-policies/core-gat... :

1. The issue has clear ‘Steps to reproduce’ in the Issue Summary. - YES
2. The fix is 'trivial' with small, easy to understand changes. - YES

Questions
1. Is the fix is easy to verify by manual testing? - YES
2. Is the fix in self-contained/@internal code where we expect minimal interaction with contrib? Examples are plugins, controllers etc. - YES
3. Is the fix achieved without adding new, untested, code paths? - YES
6. If this fix is committed without test coverage but then later regresses, is the impact likely to be minimal or at least no worse than leaving the bug unfixed? - YES

What do you think?

🇸🇰Slovakia poker10

I reviewed the MR and added some comments.

I also think this still needs an issue summary update, because these parts from IS are not implemented:

  • When Apache is reported as the web server, automatically create .htaccess files. Otherwise, do not create them and warn the user about the possible implications of this
  • New warning message in File System section of Status Report.

When manually tested this, I was unable to find the warning entry in the log (Auto-creating htaccess disabled.), as the file creation is blocked in the parent function (HtaccessWriter::ensure()), so if the auto_create_htaccess config is set to FALSE, it will never reach the code in HtaccessWriter::write(). Not sure it was the intent. Personally I think that we do not need this logging at all and it will be sufficient to create a follow-up to sort-out the errors in the status report, which are displayed if .htaccess files are missing and the auto_create_htaccess config is set to FALSE:

Private files directory - Not fully protected
See https://www.drupal.org/SA-CORE-2013-003 for information about the recommended .htaccess file which should be added to the private:// directory to help protect against arbitrary code execution.

I think it does not make sense to have it as errors anymore, when you use this new switch and disable it's creation on purpose.

Moving to NW for these.

Thanks!

🇸🇰Slovakia poker10

I think there is nothing special required for the use-case mentioned in the IS, so it seems like that if we fix this this directly in the _user_mail_notify() (see 🐛 _user_mail_notify() doesn't handle accounts without an email address Active ), no other changes will be needed here.

🇸🇰Slovakia poker10

Thanks for working on this. I think that it is a good idea to check the existence of the email address in the _user_mail_notify(), instead of in all calling functions (see the related issues in the IS).

I reviewed this and added some comments to the MR. Moving it to NW.

🇸🇰Slovakia poker10

@w01f have you tried running the SQL query mentioned here: https://www.drupal.org/node/3486109 , to check, what is causing the warning message?

🇸🇰Slovakia poker10

I propose these changes:

1. comment on new issue:

Add something like we have on s.d.o, which clearly mention not to commit anything and to read the policy. I think this is beneficial and the link is also very important. So I suggest to add:

Maintainers, please do not commit any code until directed to do so. Please review http://drupal.org/node/101497 for more information.

---------------------

2. Release window open

The message is great. I found that we have some additional information in the current message on s.d.o., which is probably worth considering:

As soon as you make the releases, update this issue with the URLs to the releases. - I asked in 📌 Decide which labels are needed for Active , if we can automate this somehow and add a comment to the gitlab issue, when a private release is created. If yes, then I think we do not need this part of the message added back. If not, then I would probably add it, as it seems valuable to track progress (at least for me).

Commit the code and create the release nodes between 16:00 UTC Tuesday and 16:00 UTC Wednesday - We have part of this in the new message (16:00 UTC Wednesday). Can we update the new message to include also the starting time (so to inform they, that they can start commiting from 16.00 UTC on Tuesdays)? If this message will be added on 8:30 UTC on Tuesdays, there is still a gap until the commit window for maintainers opens. So maybe we can update this part of the new message and add it somehow (but sorry, can't think of any good wording now): Maintainers - before the release window opens, commit the fixes to the public, ...

🇸🇰Slovakia poker10

My thoughts regarding the missing statuses:

Reviewed & tested by the community - I think that we discussed to use milestones to schedule releases. So adding an issue to a milestone can be probably considered as RTBC?

Ready for SA to be Published - We used this status when maintainers actually committed the fix and created release. Can this be automated somehow, so that we are notified about new private releases in the Gitlab issue? If so, then web probably do not need this?

Postponed - Not sure how to label such issues. Probably need to add this?

Closed (duplicate) - We can link another issues in Gitlab, but there are only three options: relates to, blocks, is blocked by. If it will be sufficient to mark it as "relates to" and close it with a comment that it is a duplicate, then we probably do not need this label.

Closed (won't fix) - Not sure how to label such issues. Probably need to add this?

🇸🇰Slovakia poker10

In D6 issues, we added the following comment:

Automatically closed because Drupal 6 is no longer supported . If the issue verifiably applies to later versions, please reopen with details and update the version.

So I suggest to use slightly modified message here, something like this (added the part what to do if the issue still applies to D10+):

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.

The link to all currently open D7 issues should be this: https://www.drupal.org/project/issues/drupal?status=Open&version=any_7 .

🇸🇰Slovakia poker10

As now also closed issues can get credits ( Grant credit for all closed issues, not just fixed issues Active ), do we need to make an exception here and disable all credits for old D7 issues which will be auto-closed by this script? I think we probably do not want credits added as there will be a lot of issues with pre-filled credits field from the past and that is not verified.

🇸🇰Slovakia poker10

The list of attendees looks correct.

🇸🇰Slovakia poker10

In the last table, pgsql module is 4%. Reading the proposed resolution, seems like that the module would qualify for removal, even if it is a DB driver. Can we exclude DB drivers modules explicitly, as I suppose we do not want to remove PostgreSQL support even if the module usage will be under the treshold?

🇸🇰Slovakia poker10

I have not used/tried Translation Bliss module yet. Can we briefly compare it with the POTX module ( https://www.drupal.org/project/potx )? Thanks!

🇸🇰Slovakia poker10

I agree with @liam morland. If we add the third checkbox, the confusing UI will be still here. So for me adding a third radio will not (fully) fix this issue.

Steps to illustrate:

1. Edit block
2. Leave the Pages textarea empty
3. Set "Hide for the listed pages"
4. See that the text on a Pages tab is "Not Restricted", but actually the block is restricted and is not displayed.

So I think we also need to fix the UI, as mentioned in the IS:

Another consideration is to ensure that the descriptive test (Not Restricted/Restricted to certain pages) in the Pages tab is correct.

, or another option is mentioned in #75. Either here, on in a separate issue.

🇸🇰Slovakia poker10

I confirm this issue on windows.

Converted the drupal_sdc-windows-3479427-8.patch to MR 11456

🇸🇰Slovakia poker10

poker10 changed the visibility of the branch 11.x to hidden.

🇸🇰Slovakia poker10

poker10 changed the visibility of the branch 3479427-bootstrap-barrio-theme to hidden.

🇸🇰Slovakia poker10

poker10 changed the visibility of the branch 3479427-drupal-sdc-windows to active.

🇸🇰Slovakia poker10

poker10 changed the visibility of the branch 3479427-drupal-sdc-windows to hidden.

🇸🇰Slovakia poker10

I think this will be fixed by 🐛 Bootstrap Barrio theme gives errors under Windows/IIS Active and that it is a general core issue (not an issue in Navigation module itself). Point 3 might be another issue and I suggest to not combine different problems in one issue, so in case you are aware of the problem and have steps to reproduce point 3, please open a new issue (if such issue does not exists yet). Thanks!

🇸🇰Slovakia poker10

Do we need a CR for this (for example for distribution developers), since we are changing the behavior for new sites?

🇸🇰Slovakia poker10

I run the test-only job. Tests are green and test-only run failed - https://git.drupalcode.org/issue/drupal-3498100/-/jobs/4539840 . Moving back to NR.

🇸🇰Slovakia poker10

Copying my thoughts from previous Slack conversations:

My personal opinion is that a lot of projects has maintenance and development status misconfigured, so it does not add much value for users. If that status would reflect for example commits, or issues activity, then it can be probably beneficial. But currently even some abandoned module can set these to "active development" without any implications. I raised this concern earlier in https://www.drupal.org/project/project_browser/issues/3267678#comment-15... 📌 Replace Wrench icon on project views with Gear icon Postponed . So I am all in favour for removing the filter.

🇸🇰Slovakia poker10

robots.txt does not seems to block /modules or /themes directories, see: https://git.drupalcode.org/project/drupal/-/blob/11.x/robots.txt?ref_typ... . This is different from D7 behavior, which blocks these directories by default.

I agree with #4 that there is probably nothing needed here. Feel free to reopen if you still think anything is needed, but will need an issues summary update in such case. Thanks!

🇸🇰Slovakia poker10

I appreciate this offer and confirm that I accept it. Thanks everyone!

🇸🇰Slovakia poker10

If author != one of the maintainers on d.o., then this is probably not dependent. Otherwise -1 to this. Commented on the related issue.

🇸🇰Slovakia poker10

I am not sure this issue is exactly the opposite of the other one, because I think we want to add the maintainers here (maybe need to update the IS/title). If the author is the owner, then I think on d.o. is always one of the maintainers. If not, I do not see a point of listing the author. But for me it will be very sufficient to list the maintainers as on d.o..

🇸🇰Slovakia poker10

I am not sure if this is regression caused by Drupal core, because when I tried to upload the same image in the core image field (not using plupload), then the image is uploaded and has correct metadata needed to pass file_validate_is_image(). Rather, it looks like that plupload module is not passing these metadata at some point and core is unable to validate the uploaded image. Can anyone confirm this?

Anyway, the workaround from #8 should be ok to use (without patching core) and even better would be to apply that only to fields using plupload upload widget, as core image fields does not seems to be affected.

🇸🇰Slovakia poker10

I quickly skimmed the MR and tried to find if we are migrating also issues followers and have not found anything. Is there a plan to keep who is following which issue, so that issue notifications are kept as they were on d.o.?

🇸🇰Slovakia poker10

+1 for adding the information about project maintainers.

Copying from the related issue - I agree, that the information about maintainer(s) is important, as they invest time to maintain the modules and should be mentioned where possible. Without the names, it may look like Drupal (or Drupal Association) is maintaining everything displayed there. We also need to think about the target audience of the Project Browser, which currently seems to be mostly Drupal CMS users = ambitious marketers (without extra experience), so they can be confused by who maintans what.

🇸🇰Slovakia poker10

To put it another way, I think that one of the benefits of Drupal CMS should be, that it should help Drupal core - e.g. bring things that are beneficial for all users to core, not to create differences in the basics (with the assumption that non Drupal CMS users will manage somehow).

Yes, the "product" term was not right, better term seems to be "system", because once you install Drupal CMS and apply recipes, it is Drupal core + contribs + custom config.

I created an issue in Project Browser: Update local task titles to match Drupal CMS changes Active .

Looking at the renamed "Recommended" tab, we are now using also the term "Add-ons" in Dashboard 🐛 "add-ons" vs. "recipes" Active , so currently one tab in Project Browser could have three different "names" - Add-ons, Recipes, Recommended. I am also curious if this tab name will need to change when other recipes will be displayed in that tab? Project Browser can eventually allow it sometimes in the future and there is no upgrade path for existing Drupal CMS users, to change the tab name back/to a different title.

🇸🇰Slovakia poker10

I was just saying, that it does not seems right to have a different UI while still in the same product - Drupal. So even if we commit this to Drupal CMS, I think there is only one way forward and that is to change it in Project Browser to have it the same. Not sure how we will be able to exaplain users of Drupal core + Project Browser (or later only Drupal core), why is their menu/tabs different from the Drupal CMS. Not saying about documentation, etc..

🇸🇰Slovakia poker10

Why we want this change to be done in Drupal CMS, instead of Project Browser? If we change it only in Drupal CMS, then users of Drupal CMS will have different tabs than users with only Drupal Core + Project Browser installed, which could be confusing.

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8, where it was fixed originally, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8, where it was fixed originally, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8 as Fixed, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8 as Fixed, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8 as Fixed, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8 as Fixed, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8 as Fixed, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

Drupal 7 is now EOL. Moving back to Drupal 8 as Fixed, so that credits are assigned correctly. Thanks everyone!

🇸🇰Slovakia poker10

The last pipeline (https://git.drupalcode.org/issue/drupal-3238194/-/pipelines/251544) has multiple failures of:

Placeholders must begin with one of the following "@", ":" or "%", invalid placeholder (!url) with string: "A user-defined date format. See the PHP manual for available options." Drupal\Component\Render\FormattableMarkup::placeholderFormat() Line: 235)

Is the issue summary up to date? Because the proposed resolution refers to #2339021-11: Incorrect call to l() in system_token_info , which is a comment year before the !url placeholder was removed in #2571695: Remove !placeholder and unsafe string return from SafeMarkup::format() .

🇸🇰Slovakia poker10

The list of attendees looks correct.

🇸🇰Slovakia poker10

I confirm this is still an issue in Drupal 11.1.1 / PostgreSQL. It is sufficient to visit the URL /%c0 to get this error (tested on Drupal CMS). We have a lot of these errors in the log on various sites.

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[22021]: Character not in repertoire: 7 ERROR: invalid byte sequence for encoding "UTF8": 0xc0 0x20: SELECT "base_table"."id" AS "id", "base_table"."path" AS "path", "base_table"."alias" AS "alias", "base_table"."langcode" AS "langcode" FROM "path_alias" "base_table" WHERE ("base_table"."status" = :db_condition_placeholder_0) AND ("base_table"."alias"::text ILIKE :db_condition_placeholder_1) AND ("base_table"."langcode" IN (:db_condition_placeholder_2, :db_condition_placeholder_3)) ORDER BY "base_table"."langcode" ASC NULLS FIRST, "base_table"."id" DESC NULLS LAST; Array ( [:db_condition_placeholder_0] => 1 [:db_condition_placeholder_1] => /� [:db_condition_placeholder_2] => en [:db_condition_placeholder_3] => und ) in Drupal\path_alias\AliasRepository->lookupByAlias() (line 94 of core/modules/path_alias/src/AliasRepository.php).

Drupal\Core\Database\StatementWrapperIterator->execute() (Line: 658)
Drupal\Core\Database\Connection->query() (Line: 240)
Drupal\pgsql\Driver\Database\pgsql\Connection->query() (Line: 520)
Drupal\Core\Database\Query\Select->execute() (Line: 145)
Drupal\pgsql\Driver\Database\pgsql\Select->execute() (Line: 94)
Drupal\path_alias\AliasRepository->lookupByAlias() (Line: 131)
Drupal\path_alias\AliasManager->getPathByAlias() (Line: 37)
Drupal\path_alias\PathProcessor\AliasPathProcessor->processInbound() (Line: 70)
Drupal\Core\PathProcessor\PathProcessorManager->processInbound() (Line: 142)
Drupal\redirect\EventSubscriber\RedirectRequestSubscriber->onKernelRequestCheckRedirect() (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}() (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher->callListeners() (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher->dispatch() (Line: 159)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 709)
Drupal\Core\DrupalKernel->handle() (Line: 19)
🇸🇰Slovakia poker10

There is no mention of the "Add-on" term in the Drupal CMS documentation: https://new.drupal.org/docs/drupal-cms .

We are already using term "Extend", so probably something like "Extend the site functionality", or something like this would be much better, until the term "Add-on" is widely used in Drupal CMS/Drupal core. Otherwise I agree that it has potential to create confusion.

🇸🇰Slovakia poker10

Would be great the other way around also.

Done.

🇸🇰Slovakia poker10

For the reference, there is an issue where Drupal 11 PostgreSQL requirements were discussed: 📌 [11.x] [policy] Require PostgreSQL 16 for Drupal 11 Fixed .

Also there is this issue for future Drupal versions and its database versions: 🌱 [Policy] How to select the minimum required database versions Active

Production build 0.71.5 2024