FWIW, there was already discussion about the "By" vs "Co-authored-by" in comments #7 to #37 and "Co-authored-by" was selected. I personally prefer "By", so +1, but I think we are getting in circles here.
Re #20 and #31 - I think this issue should be fixed regardless of the 🐛 Don't display OEmbed error to anonymous visitors when resource stops being available Needs work , because the error should not be visible to anonymous users. The other issue's MR does not fix the message displayed for anonymous users, so let's keep both issues open.
@marcus_johansson I am curious, why this has been closed, if it was not fixed in 1.x branch and it is still not fixed/rewritten/removed in 2.x branch? Why was not the issue version field updated to the 2.x branch and left open until it is actually resolved?
We invested significant time testing the AI module(s) in Drupal CMS prior to, or around the initial Drupal CMS release. Closing this issue (not sure if there are also others with the same outcome) as “outdated” now feels somewhat discouraging (if done without proper resolution and attribution). It just does not seem right to me. Thanks!
I still think that having common statuses for each project in the Drupal ecosystem is a good thing and it good also for contributors, because they do not need to study specific docs / workflows for each project and can just use one global docs like this https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... → everywhere. That is consistent and predictable. Imagine that some project will delete all status labels and introduce some weird custom labels. I am not seeing how that could be better.
The plugins are not considered a public API → , but I am curious if we should deprecate it first as per https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... → anyway? It is also a public property which could probably be used in some contrib modules (even though I have not found any evidence for that so far). Thanks!
Thanks for working on this.
I tested the MR manually and it fixes the problem. I have one question though - why the machine name cannot end with a dot? I have not experienced any issues when a dot was a last character, only when the dot was a first character. Can we add additional explanation to the IS, why we are removing the dot in the end, or sanitize only the dot at the beginning? The title also suggest that we are sanitizing only a dot at the beginning. Thanks!
Thanks for working on this.
I reviewed this issue and MR and I personally think, that we should not update these hardcoded tables in the current state, but instead we should try to convert them to views - which would give us much more options to customize them (also for site owners themself). I found that such issue already existed: ✨ Convert "Top 'access denied' errors.", "Top 'page not found' errors" and Top search phrases to views Needs work .
Regarding this issue - since it is adding a new column to the table, I think that is an UI change and as per Usability core gate → it needs screenshots in the IS and probably also a feedback from the UX Team about the new column title. There is also another similar issue to update the other column title here: 📌 Rename the table headers on the watchdog 403/404 controllers Postponed: needs info
I am postponing this issue on ✨ Convert "Top 'access denied' errors.", "Top 'page not found' errors" and Top search phrases to views Needs work . Thanks!
Adding a related issue. I think that if we convert these pages to views, then we will have much broader options how to customize/modify it (also for sites admins themself). Not saing this is blocked by that related issue though.
I agree with @ressa and reopening this feature request.
There is another issue to add additional column to the listing ( ✨ Include most recent date in page not found and access denied reports Active ), but I am not sure we should be doing that, given it is a static hardcoded table, without any pager or other possible customization options for site admins. Instead we should consider creating these pages as views, which seems to be a correct approach - then, almost any customization will be possible, even without additional core changes.
It does not seems to be hard to create a similarly working view - for an example view for "top 404 pages" see the config I attached (it is an export of the view we have used when replaced the old pages). It is not fully complete to be committed as is, but it can serve as some starting point for this issue.
Thanks!
Thanks for working on this!
I am curious if we need to run daily jobs with all three PostgreSQL versions (16, 17, 18), given we run daily jobs only for one MySQL/MariaDB/SQLite versions? Maybe we could change the existing PostgreSQL 17 daily job to PostgreSQL 18, to have the default and latest versions tested daily and not add a third one?
For the reference, here is a link to the original security issue: https://security.drupal.org/node/70884 (will work only for Drupal Security Team members) - the token lenght was discussed very briefly.
Thanks for working on this.
Currently the token is long 8 characters. The proposed solution will extend it to 40+ characters and will change URLs on existing images derivatives on sites. It can cause issues, because as far as I know, for example Google indexes the full URL of images, including query string (see for example this article).
It would be great if we can get some numbers about the probability to effectivelly bruteforce the current implementation to see, if there is a real chance of doing that and that it is really beneficial for sites to increase it/increase it that much (for example when considering a password length, from a specific point, there is minimal to no benefit having it longer, as it would mean practically no extra security in the current real world conditions). I think we should not make the token longer as really needed.
Moving back to NR to discuss it a bit more. Thanks!
Thanks for working on this.
I think that the proposed change can cause issues on some sites. As per https://developers.google.com/search/docs/crawling-indexing/robots/robot... , the Disallow: /search will match anything starting with /search, so it has a potential to also block any content with URL alias starting with that string (for example a news with an alias of /search-for-donations, etc.). Based on this, moving back to NW.
Also this is probably more like a Feature request than a Bug, given there is no issue with the core itself.
Once this is ready, I am curious if we can see a migrated test project somewhere (even in the staging/test env), to help with testing before doing the first production migration of the DA projects? That could probably help identify issues before doing first irreversible production migrations.
And should this issue 🌱 Using GitLab labels for issues on Drupal projects Active be a blocker too, since we probably need that decided before the first migration? Thanks!
Personally, I agree that keeping old NIDs as GitLab IDs seems more beneficial than keeping authors of issues. Great we can solve it at least this way.
Looking at my earlier comment #22 (and #23) - is there a decision whether we want to keep/migrate also the comments redirects? Seems like there are at least two places in 11.x branch (1, 2), where such a link is used. Not sure how many are used in issues' comments itself (can we check it in the database?).
Also have another question - currently the redirects on i/NID and links on node/NID are universal and are working even without knowledge of the specific project, in which the issue is created. I suppose that if we move an issue to a different GitLab project after the migration, the redirect from node/NID would still work to a correct (new) GitLab project?
And speaking about this (but this question is not related to the migration itself), it seems like that for all new issues created in the GitLab after the migration, we will loose the ability to find such issue just by entering the ID somewhere (without knowledge of the specific project)? Presumably, this is also because issues in GitLab have separate numbering in each project?
Thanks!
In D7 version of the module, there was:
$menu_item['xmlsitemap']['access'] = $menu_item['access'] && !$menu_item['external'] && !$menu_item['hidden'];
In the modern Drupal version, there is ony:
$entity->xmlsitemap['access'] = $loc && $access;
So yes, it seems like the module is not checking if the menu link is hidden/disabled. Only if user has access to the underlying link. I think that something like checking $entity->isEnabled() could work here.
I discussed this with @catch on committers Slack channel.
We both think that removing all messages without knowledge what else could be there is a bit risky. Even if the risk is small, it could affect some sites UX negatively, if any unrelated message will be removed accidentally. Another disadvantage is, what @ivnish asked in #91, that there is no way to remove just the one message, we need to change. Adding another message (and have two here) does not seems right.
I re-read the whole issue and it seems like, that the initial proposal was to add a description to the message textarea and only later it was proposed to add also a message on form submit. Based on this, the usability team proposed to remove the description and stick only with updating the submit message(s). However, the initial intention was to add only something simple like this: To display the maintenance mode message to all site visitors, flush the page cache on the Performance Settings page..
When we started mixing it with the submit message, it ended up as: The site is now in maintenance mode. The maintenance mode message will not show on already cached pages. To clear the cache, visit the Performance page.. Which is different from the original meaning, which was just to inform a user about caching. The current message also informs a user that the maintenance mode is turned on, which I think should not be in the scope of this issue, because we already have a similar message informing users about the active maintenance mode, see: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co... (Operating in maintenance mode...). This message is intentionally excluded from the system.site_maintenance_mode route (see
#184926: Incorrect message displayed when bringing a site back online →
). I do not think that we should add the information about the maintenance mode status back to the form, as users already see whether the checkbox is checked, or not. Or at least we should not do it in this issue.
Considering all this, I propose to return back to the original proposal - do not change submit message(s) and just add a simple and easy-to-read description to the message textarea (and do this only if page_cache module is enabled). Something like The message will not show on already cached pages without clearing the page cache.. I propose a bit different wording for the last part, so that we do not need to deal with the possible different permissions of the user (see #90.2). Another option discussed on a Slack was to put all information in the form, depending on the current state.
I am adding Needs usability review tag again and will ask for another opinion from the usability team based on these new findings.
Thanks!
My intention was not to hold this issue, but part of the RTBC issues review process → is evaluating the scope of the issue. I definitely think that changing the documentation for a same parameter in two functions directly related should be done in one issue, not in two issues, because it will create unnecessary work for maintainers a reviewers, see: https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... →
If we take a look at \Drupal\Core\Mail\MailManagerInterface::mail() and \Drupal\Core\Mail\MailManager::doMail(), the documentation is mostly the same for each parameter. If we update only one of these functions, we will end up in inconsistent state.
Regarding a missing test - yes, there is a mention in the IS, that multiple addresses in the reply-to header works, but there is no proof for that (and also there is a comment #11, which states the opposite). Ideally, there should be either a test for this (which I have not found) or at least a confirmation (for example link to the documentation), that all implementations of \Drupal\Core\Mail\MailInterface::mail() supports it (at least the two implementations, which are supported by the core - PHP mail and Symfony mailer).
Thanks!
I have added one minor comment about the "read-only" vs "non-executable" wording.
Thanks for working on this.
I manually tested the MR since there are no tests to prove that the message is actually shown. It worked as expected. Anyway, I think we should extend the existing SiteMaintenanceTest to check if the code is working as expected and to prevent regressions in the future, especially if we are "blindly" removing original status message(s).
I also have some additional questions:
1. Aren't we worried that we could potentially remove more messages when using this code \Drupal::messenger()->deleteByType(MessengerInterface::TYPE_STATUS);? For example if there will be any custom messages included. We are not checking the actual message being removed.
2. Route system.site_maintenance_mode has permissions of administer site configuration+administer software updates, which means that if you have only administer software updates, you still have access to the maintenance mode form. But if you do not have administer site configuration permission, the newly added link to the Performance page will not work. Are we OK with keeping it there even if the user won't have access to it?
poker10 → created an issue.
Thanks for working on this.
It seems like the same parameter with a same docs (Optional email address to be used to answer.) is also in core/lib/Drupal/Core/Mail/MailManager.php (see: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...).
\Drupal\Core\Mail\MailManager::mail() passes the $reply parameter directly to the \Drupal\Core\Mail\MailManager::doMail(), so I suggest that we fix both occurrences here in this issue, as these are very closely related.
I briefly looked at the tests and it does not seems like that \Drupal\Tests\system\Kernel\Mail\MailTest::testFromAndReplyToHeader() is testing multiple emails in the reply-to header. So I suggest we extend the test and test the support for multiple email addresses, since we are putting it to the docs.
Thanks!
If we do not consider "co-authored" as a potential issue with copyright, as mentioned in #3540547-8: Add a UI to change "role" values for each user in the footer → , then for me either By, or Co-authored-by looks good. I would not add emails, as discussed above.
The updated text looks good, thanks!
The list of attendees looks correct, thanks!
benjifisher → credited poker10 → .
The list of attendees looks correct.
It looks like WP changed the algorithm to bcrypt in february and they seems to pre-hash passwords: https://core.trac.wordpress.org/changeset/59828 (line 2706).
If we go the way as proposed in the MR, then I think it will be better if we not fail silently in case the password will be longer - so agree with #14 that we should add at least some information for user.
benjifisher → credited poker10 → .
Indeed, we should add the ability to auto-detect in a follow-up
I still see the auto-detect feature in the MR, is that correct? Can we remove it for now, so that users are not confused how the values get there? If we would like to keep the auto-detect for now, I think we should bring back at least the descriptions to explain, that values were auto-detected and also mention the path which was autodetected. This can also help in case someone moves the site - then there will be the actual auto-detected link in the description and users can use it if needed (they will not need to search it manually). Anyway, the auto-detect feature is not mentioned in the IS in the proposed resolution.
In #36.4 I posted a bug on windows. Since we did not drop windows support yet (#3436617), is this worth exploring further? For example on my testing windows, composer was detected, but rsync not and I was stuck, because the auto-detected composer value did not pass the validation on the submit.
This seems like a (significant) UI change, so I think that even without an option to reset, the current approach still needs to be approved as per Usability core gate →
Based on these, moving this back to NR. Thanks for all the work here!
But if we include the module it eliminates the module selection process by the site template creator and consolidates usage to that module.
I do not think it is a good idea to explicitly select a specific module from a set of similar modules, unless Drupal CMS needs it. We have a big ecosystem of modules and maintainers put a big effort in maintaining these, so we should not make choices for our users, if we do not need (speaking generally, not about this specific case, where @svendecabooter is the maintainer of RoleAssign).
If you download and extract it, the vendor directory contains mostly empty folders, so this looks like a bug.
Wrong branch was selected when creating the MR. Updated that and moving to NR.
poker10 → changed the visibility of the branch 3529392-frontpage-settings to hidden.
quietone → credited poker10 → .
Thanks for reporting and working on this. There is a lot of unrelated changes in the MR because it targets 8.x-1.x branch. We need to have MR against 2.x branch. Also please keep open only one MR, as currently it is not clear, which one is ready for review (!63 or !64).
Thanks for reporting this.
Valid values as per specification (https://www.sitemaps.org/protocol.html) are:
always
hourly
daily
weekly
monthly
yearly
never
There is no "quarterly". Closing this as won't fix.
Thanks for reporting and working on this. There is a lot of unrelated changes in the MR because it targets 8.x-1.x branch. We need to have MR against 2.x branch.
poker10 → created an issue.
This seems to be a related issue: 🐛 Not checking "Allow multiple values" in contextual-filter, with PostgreSQL, results SQL error Active
But the Related blog posts display in the Blog view does not have Allow multiple values checked in the Content: Tags (Default: Taxonomy term ID from URL) contextual filter. Once the option is checked, the error is gone. So I think this can be fixed in Drupal CMS as well with adjusting the view config - if we set Allow multiple values by default.
@hestenet I am curious if there has been any progress with this issue? Thanks!
The list of attendees looks good, thanks!
Adding attendees according to the DST private notes.
benjifisher → credited poker10 → .
pameeela → credited poker10 → .
It seems like the decision may affect Gin toolbar module, which is used on 60k+ sites and offers horizontal and vertical menu. If removed, it will cause upgrade issues for sites which are currently using it. Is there any plan to keep/or add back the possibility to have also the horizontally oriented menu in such case? Navigation itself currently does not support horizontal menu, only vertical, which may be quite restrictive, given the original core's Toolbar module offers both options (see the comment #88 📌 New Toolbar Roadmap: Path to Beta & Stable Active ).
What about existing forks/merge requests, which are associated with d.o. issues? Will these be migrated too?
According to my testing from 🐛 InvalidComponentException when Workspaces UI and Navigation modules are enabled Active , the error only appears if Workspaces UI is enabled. It is not sufficient to enable just Navigation and Workspaces to trigger the error. So I am updating the title and adjusting the priority as it is pretty easy tu run into this.
Yes, it looks like a duplicate. Thanks. Let's continue in the older issue.
Oh, sorry I overlooked this. Maybe I was focused mainly to steps to reproduce and that the issue is still open, so did not read the rest correctly. Created a new issue: 🐛 InvalidComponentException when Workspaces UI and Navigation modules are enabled Active . Thanks!
Sorry, I overlooked the specific error in the #3511374, but originally I meant this new issue: 🐛 InvalidComponentException when Workspaces UI and Navigation modules are enabled Active as a stable blocker in #89.
I think this should be a Navigation stable blocker.
I cloned 11.x, installed standard on MySQL, enabled Navigation, enabled Workspaces+UI and still get:
Drupal\Core\Render\Component\Exception\InvalidComponentException: [navigation:toolbar-button/icon] NULL value found, but an object is required. in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 234 of /data/test/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).
Drupal\Core\Template\ComponentsTwigExtension->doValidateProps(Array, 'navigation:toolbar-button') (Line: 106)
Drupal\Core\Template\ComponentsTwigExtension->validateProps(Array, 'navigation:toolbar-button') (Line: 47)
__TwigTemplate_94743d30f511513d29aca6f93557776f->doDisplay(Array, Array) (Line: 402)
Twig\Template->yield(Array) (Line: 151)
__TwigTemplate_3a476adc6e0e556dd3a0a8fb64e407f3->{closure:__TwigTemplate_3a476adc6e0e556dd3a0a8fb64e407f3::macro_menu_items():77}() (Line: 2106)
Twig\Extension\CoreExtension::captureOutput(Object) (Line: 77)
__TwigTemplate_3a476adc6e0e556dd3a0a8fb64e407f3->macro_menu_items(Array, Object, 0) (Line: 54)
__TwigTemplate_3a476adc6e0e556dd3a0a8fb64e407f3->doDisplay(Array, Array) (Line: 402)
Twig\Template->yield(Array, Array) (Line: 358)
Twig\Template->display(Array) (Line: 373)
Twig\Template->render(Array) (Line: 51)
Twig\TemplateWrapper->render(Array) (Line: 34)
twig_render_template('core/modules/navigation/templates/navigation-menu.html.twig', Array) (Line: 380)
Drupal\Core\Theme\ThemeManager->render('navigation_menu', Array) (Line: 493)
Drupal\Core\Render\Renderer->doRender(Array, Object) (Line: 246)
Drupal\Core\Render\Renderer->doRenderRoot(Array, Object) (Line: 142)
Drupal\Core\Render\Renderer->{closure:Drupal\Core\Render\Renderer::renderInIsolation():141}() (Line: 623)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 141)
Drupal\Core\Render\Renderer->renderInIsolation(Array) (Line: 168)
Drupal\Core\Render\Renderer->doRenderPlaceholder(Array) (Line: 725)
Drupal\Core\Render\Renderer->{closure:Drupal\Core\Render\Renderer::replacePlaceholders():724}()
Fiber->start() (Line: 733)
Drupal\Core\Render\Renderer->replacePlaceholders(Array) (Line: 258)
Drupal\Core\Render\Renderer->doRenderRoot(Array, Object) (Line: 142)
Drupal\Core\Render\Renderer->{closure:Drupal\Core\Render\Renderer::renderInIsolation():141}() (Line: 623)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 141)
Drupal\Core\Render\Renderer->renderInIsolation(Array) (Line: 168)
Drupal\Core\Render\Renderer->doRenderPlaceholder(Array) (Line: 725)
Drupal\Core\Render\Renderer->{closure:Drupal\Core\Render\Renderer::replacePlaceholders():724}()
Fiber->start() (Line: 733)
Drupal\Core\Render\Renderer->replacePlaceholders(Array) (Line: 258)
Drupal\Core\Render\Renderer->doRenderRoot(Array, Object) (Line: 142)
Drupal\Core\Render\Renderer->{closure:Drupal\Core\Render\Renderer::renderInIsolation():141}() (Line: 623)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 141)
Drupal\Core\Render\Renderer->renderInIsolation(Array) (Line: 112)
Drupal\Core\Render\Renderer->renderRoot(Array) (Line: 253)
Drupal\Core\Render\HtmlResponseAttachmentsProcessor->renderPlaceholders(Object) (Line: 74)
Drupal\big_pipe\Render\BigPipeResponseAttachmentsProcessor->processAttachments(Object) (Line: 45)
Drupal\Core\EventSubscriber\HtmlResponseSubscriber->onRespond(Object, 'kernel.response', Object) (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::{closure:Symfony\Component\EventDispatcher\EventDispatcher::optimizeListeners():241}(Object, 'kernel.response', Object) (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher->callListeners(Array, 'kernel.response', Object) (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher->dispatch(Object, 'kernel.response') (Line: 216)
Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object, Object, 1) (Line: 204)
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: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 90)
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: 53)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 715)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
The same result also on Simplytest.me.
Is this really fixed in 11.2.x?
Dec. 16 is incorrect. The correct date for the 2026 December window is December 9.
I think the intent was to set this to a core security release window, which is usually the third Wednesday of the month and that is Dec 16th 2026.
Personally, I prefer more setting up a fixed date, because that will remove any uncertainties. The currently proposed text seems a bit confusing for me, at least the part
in early December 2026, when Drupal 12 is released
, as Drupal 12 could be released also in June.
I agree that we technically cover all code on git.drupalcode.org, if the project is opted into security advisory coverage and has stable release. So also recipes (https://new.drupal.org/browse/recipes) and general projects ( https://www.drupal.org/project/project-general → ).
We probably need to update the wording in https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... → , but in the SA policy ( https://www.drupal.org/drupal-security-team/security-advisory-process-an... → ), the project types (modules, themes, distributions) are not explicitly mentioned and the policy is not restricted to these types.
This looks good and the link to list all issues by project in the issue description is great, thanks!
One additional question came to my mind - is it possible to use the Advanced search to search also by labels? For example in the Security team triage doc, we are using links to s.d.o. with already filtered issues by project and status (for example all core issues needs review). Is is possible to include label here https://git.drupalcode.org/search?group_id=183118&scope=issues&search=%2... somehow?
+1 to set an explicit D10 EOL date, so that it is predictable. I think December is a good idea, regardless of the D12 release date. However, can we make the bold part more specific?
Define early December 2026 as the definite EOL time for Drupal 10.
If the intent is to set explicit date, can we set it to a specific date and not say "early..."? I agree with @damienmckenna, that setting a specific date may be better (whether it will be 2026-12-31 or some other date in December). Otherwise it is still a "moving target" - though less, but still at least until a certain date closer to the release, when a specific date of the D12 release will be published.
@quietone Thanks! I think we still need to update other screenshots (from the SettingsForm), so keeping this at Needs work for this and also for other issues from #36.
@catch I think that if we base the required state on the check done by SymfonyExecutableFinder/PhpTuf , the field will become not required as soon as the executable is detected again (sorry if I used the wrong word was/is). My idea was, that if the specific executable is not auto-detected, then the required flag should be set until it is auto-detected. In case the specific excecutable is auto-detected, it can be kept as is, without the required flag, so that you can set custom path, or revert to the auto-detected value. I just think that resetting the executable path back to NULL (default value) should not be allowed if the auto-detected path is not available at the moment, because it will reset you to the non-working state regarding that specific executable.
It would be also great to update the IS, as the images in User interface changes are outdated (for example there is no (auto-detected) in placeholder, the details element does not contain info that you can clear the value to return back to auto-detect and there are additional status/warning messages).
I tested the MR manually and found some issues:
1. Initially, package manager auto-detected my composer, but not rsync. The rsync field was therefore required. I entered manually the path to the rsync executable and the field was no longer required after saving the form. Therefore I was able to delete the value again - which is probably not what we want, since it was not auto-detected initially? I think it will be better to keep the required flag always, if the executable was not auto-detected to not allow removing the value.
2. When you save the form with a value in a field which was not auto-detected, after the form is submitted, you get duplicate messages, like:
Status message
The path to Composer was automatically detected.
The configuration options have been saved.
The path to rsync was automatically detected.
Warning message
The path to rsync could not be automatically detected.
So the info about rsync path was duplicated. The same issues happened when I removed the rsync path (as mentioned in point 1).
3. When I set the path to rsync manually, I see the message:
The path to rsync was automatically detected.
Which is not correct, as I entered the value manually and it was not detected automatically.
4. On windows - composer path was detected as C:\ProgramData\ComposerSetup\bin\composer.BAT. In windows enviroment settings (PATH), there is C:\ProgramData\ComposerSetup\bin and the composer executable file exists physically with a lowercase extension (not uppercase). Not sure if that is a problem (and if this is caused by SymfonyExecutableFinder). What is a bit worse is, that even when the PHP docs mention, that is_executable() function will detect .bat files as executables (see https://www.php.net/manual/en/function.is-executable.php - for BC reasons, files with a .bat or .cmd extension are also considered executable), when I try to save the form with manually entered C:\ProgramData\ComposerSetup\bin\composer.BAT or C:\ProgramData\ComposerSetup\bin\composer.bat, it fails the IsExecutableConstraintValidator ( "C:\ProgramData\ComposerSetup\bin\composer.bat" is not an executable file. ). This is probably not good for windows users.
5. Can we consider adding an info somewhere, that removing the value will "reset" the config to the auto-detected value?
Moving this to Needs work for these. Thanks!