🇦🇺Australia @mstrelan

Added a first pass, seems to work from rudimentary testing. Needs test coverage.

We already have \Drupal\Core\Config\Entity\ConfigDependencyDeleteFormTrait::addDependencyListsToForm. We need to add the confirm form step as per \Drupal\system\Form\ModulesUninstallForm::submitForm and \Drupal\system\Form\ModulesUninstallConfirmForm::buildForm.

Updated the IS with issue template and added steps to reproduce.

I don't use the ban module, but I'm motivated to assist in removing it from core. So I'd be happy to maintain a 1.x version, but would need to find others to maintain the proposed 2.x version.

To bike-shed here I would simply call it createItems or possibly createMultiple, similar to EntityInterface::loadMultiple.

I'm not sure what is the BC way of adding methods to an interface. We could create a new interface that extends QueueInterface and provide a trait that has a default implementation with a simple loop.

It looks like we could easily refactor \Drupal\Core\Queue\DatabaseQueue::doCreateItem to insert multiple items at once with a bit of massaging. In fact, the simple_sitemap module already does this in SimpleSitemapQueue. Not quite sure about the return value though.

At a glance the redis client already allows preprending multiple items, so redis module can easily make use of that too.

In core there are not a lot of places that could make use of this. I've identified the following:

• _batch_populate_queue - multiple batch operations
• locale_cron_fill_queue - same as above
• \Drupal\media\Entity\Media::postSave - download thumbnails for multiple translations

So we're probably only saving a handful of queries there. But there are more places in contrib that could also make use of this. A few I see in the project I'm currently working on:

• entity_reference_revisions_entity_delete
• \Drupal\linkchecker\LinkCheckerService::queueLinks
• \Drupal\workbench_email\EventSubscriber\WorkbenchTransitionEventSubscriber::onContentModerationTransition

I'm not sure that system.module is the right component though, so changing to base system.

There's not much to go on here. The IS refers to a YouTube video without any links to said video.

If anything, maybe hold on to this issue when Big Pipe is ready for some spit and polish.

Is that time now?

When this issue was opened frontend frameworks were not as popular as they are now. Perhaps decoupled Drupal with a React frontend solves this for some. We also have things like HTML-native lazy loading attributes that didn't exist in 2015.

I think to progress this issue we need some clarity. Perhaps let's start with the standard issue template.

I suspect this is a problem Project Browser → aims to solve, and is unlikely to be change in Drupal core.

Changing component. While system.module provides the module install form, I think extension system is more appropriate.

FWIW adding these to .info.yml files is probably not particularly discoverable, so we'd need to flesh out how this information is used.

As far as I can see this should have been closed as a duplicate in August 2013 as per #6

#1355292: Come up with better alternatives for groupings on the modules page →
📌 Introduce tags[] in module.info.yml file to categorize modules by provided functionality. Postponed: needs info

Probably something like Symfony Messenger + Drupal: Realtime Queues and Cron → is better for this anyway.

I think first with composer and now with package manager and project browser this is a "won't fix".

Is this still applicable? If so, please provide clear steps to reproduce. I don't think it's related to system.module, the queue system is in core/lib/Drupal/Queue. I couldn't find a "queue system" component so I guess base system will have to do.

BulkForm was moved to views.module in #2916451: Move everything related to Bulk Form to Views module → , updating component

Since we have 📌 Port password-hash.sh to a console command Needs work and #2289447: Port rebuild_token_calculator.sh to a console command → should we close this and focus on those? Or postpone this on those ports and use this issue to remove them? Either way, it's got nothing to do with system.module so changing the component to "base system" to match those other two issues. Probably "other" would be suitable too.

Given the lack of interest here I think we can safely close this "won't fix". The built-in phpinfo page is well known amongst PHP developers and is not intended to be used by ordinary users. I don't see any value in changing the way it looks or works.

Didn't mean to change the component, but thinking it probably belongs here anyway.

Given there is no response from #12 in 2017, I think we should close as a duplicate of ✨ Allow multiple field widgets to not use tabledrag Needs work .

Is this still applicable or has Apache and/or Drupal changed in the last 7 years?

This is probably a duplicate of ✨ Add default date formats without time Closed: duplicate but this has obviously had much more recent discussion. Perhaps someone could credit @dead_arm for the original issue I've just closed.

This issue obviously came first, but seems this will be implemented in 📌 Add date formats without time Active .

I'm not sure this is as relevant anymore since #2401035: items_per_page in node.settings is no longer used → . There is no longer an option for "Number of posts on front page" and the current help text is just "Specify a relative URL to display as the front page."

I can't think of any site I've ever worked on that has used the promoted to front page feature, and it seems it's falling out of favour. There are some related issues below:

#29338: Hide Promoted/Sticky fields by default in Form display →
📌 "Promoted to front page" for new content types should default to Un-Checked Needs work
#2514794: Frontpage view is confusing when only one node is promoted to the default front page →
#2029199: Rename the 'Frontpage' view to 'Latest content' or 'Featured content'. →

I think instead of adjusting help text on the site information settings page we could shift focus on the welcome to Drupal block you get on the default front page instead. Setting to PMNMI in case anyone has different ideas.

Not sure if we need to do anything here or should just close it. The idea was to find a use case so we could add the dependency, but now it's been added anyway so no need to do this. Unless there is a rector for this maybe?

There is nothing to review. Changing status for #17.

Looks like there is already 📌 Remove usages of .js class from core Needs work . Shall we close this then?

@andypost see #34-39

This is not the answer

Considering token contrib module has [current-page:page-number] is this still required? Setting to PMNMI to determine if people still want this in core.

Wondering if this link still needs to exist now that we have Project Browser → .

This may also be relevant 📌 [PP-3] Figure out what to do with the install/uninstall modules page Postponed

It's fairly clear that composer is the way forward here. Contrib modules can already declare their PHP dependencies in composer.json. For core, the granularity is nice, but I think we can just add to the core composer.json if extensions are required. Are we happy to close this?

See also:
✨ Replace .info.yml with composer.json for extensions Postponed
#3005229: Provide optional support for using composer.json for dependency metadata →

Is there much of a use case for this in core? Maybe it's better to use contrib such as https://www.drupal.org/project/theme_switcher →

Try this rector rule:
https://getrector.com/rule-detail/remove-useless-var-tag-rector

I assume this is referring to the textfield on /admin/modules. There is a similar filter on /admin/structure/views that works in the same way. Tagging for usability review to decide if this is something we should do.

BulkForm was moved to views module in #2916451: Move everything related to Bulk Form to Views module → , moving to correct component.

This is a duplicate of #2701851: The 'system.db_update' route should restrict access via the 'access_check.db_update' service →

Not sure what this is postponed on so I'm marking it as Active.

FWIW the IS suggests that the output of the two templates is the same, but it's not. As per #3 one template has "layout-column" classes and the other has "left" and "right" classes, that don't actually work.

Also pointing out that admin-page.html.twig is overridden in Claro, but system-admin-index.html is not, and it seems the main difference is the lack of a wrapping clearfix class.

I think we should deprecate system-admin-index.html and use admin-page.html.twig for the /admin/index route. This would also restore the two column layout that presumably once worked.

@quietone re #18 stdClass would be covered by this part of the PHPDoc Types page:

A fully-qualified name (FQN) like \Foo\Bar\Baz, or a relative name like Baz resolved based on the current namespace and use statements can be used.

Is this still relevant, given the lack of interest in the past 9 years? As @dawehner pointed out, you can just write your own ThemeNegotiator.

Looks great

This is a duplicate of 📌 Move Front page & Error page settings out of "Site information" and into their own respective config pages. Needs work but with a narrower scope. I'm not sure which one makes more sense, so leaving them both open.

Let's update the issue summary to also mention the entity.image.preview route. I agree with #40, no need to deprecate.

Not sure I agree this is necessary, but if we do it I think we should have a more generic name than FrontAndErrorPagesForm, perhaps SystemPagesForm? Or we could rename "Site Information" to be more generic so that these settings belong too. Either way, it needs to be rerolled in to an MR against 11.x. The IS needs updating with the template, screenshots, etc

Major version upgrades and QA should be performed in non-prod environments. As per #2, there is a contrib module if you still want to do this. Given the lack of updates in the 15 years this has been opened I'm going to go ahead and close this. Feel free to reopen if you disagree.

What is this postponed on? Changing to PMNMI for that information. Is this still relevant with project browser and package manager? It may also be replaced by ✨ Replace .info.yml with composer.json for extensions Postponed . Arguably this could move to the "extension system" component, although system.module is what provides the module install form.

In light of 📌 [meta] Tasks to deprecate the Contact module Active I don't think core will be updating its APIs just for contact module. Restoring original title and component so it can move to contrib along with the module.

The classes were removed intentionally in #2407743: Remove classes from system templates s*.html.twig → . When this issue was opened it was still common to use classy as the base theme, so this may have seemed like a bug, but now we have starterkit_theme which includes the 'messages--' ~ type class.

Is this something we still want to do? It's been almost 10 years since the last update. My initial thoughts are this can be closed (won't fix).

I've updated the screenshots to show the current situation. The main difference is the the install form shows all dependent modules whereas the uninstall form only shows installed modules.

I'm not sure how relevant the comments are about using the information when considering updates, now that this is mostly handled by composer or package manager anyway.

Opened 📌 Deprecate RSS usage in core Active to discuss if we want to deprecate RSS more broadly in core

Opened 📌 Deprecate RSS usage in core Active to discuss if we want to deprecate RSS more broadly in core

mstrelan → created an issue.

It's been almost 10 years since the last update, is this still relevant? According to https://caniuse.com/?search=%40viewport this is only supported in IE and Opera Mobile. See also https://github.com/w3c/csswg-drafts/issues/4766. I was going to close it but perhaps there is a new way to do this if it's still required, so postponing it instead. Also moving to the CSS component.

I agree with slimming down system.module but I'm not sure that date format UI warrants its own module. I had a look at all the other forms in system.module and I guess it makes sense in that this is the only EntityForm in the module. I also wondered what other forms could be removed and RssFeedsForm jumped out at me, but then I found it's already slated for removal in 🐛 Views RSS view mode settings are completely broken Needs work .

I guess if we do want to proceed we need to:

1. Deprecate the old DateFormatFormBase
2. Update hook to enable date_format_ui on existing sites
3. Enable in Standard and Umami
4. Consider if this should be enabled in recipes

#29/#30 seem like unrelated follow ups to me. The object/stdClass example is in the original text and unchanged in the proposed text.

Maybe install system is a better home for this

I don't really understand this issue, enter has always submitted forms. If for some reason we decide we want to disable it then why should it be limited to the modules form? You can do the same damage on the Permissions form, for example. Since it's been almost 10 years since the last comment I'm setting to PMNMI to see if anyone is still interested in following this up.

Is this still relevant? It's been almost 10 years since the last update and a lot has changed since then. It's always great to profile and reduce memory consumption, but it's unclear exactly where this issue is headed. Marking PMNMI for anyone involved to provide direction / next steps.

I agree that global theme settings are confusing. For most sites, a lot of the settings aren't really applicable anyway. For most sites I've worked on there are no user pictures, nor are there comments. It also doesn't make sense for "User verification status in comments" to be configurable per-theme (or in global theme settings). This should be comment.module settings. I don't know any site that would set the shortcut icon (favicon) per theme either, it's always global.

My suggestion would:

1. Remove "User pictures in posts" and "User pictures in comments" from global settings. They can stay in themes that support them.
2. Move "User verification status in comments" to /admin/config/people/accounts as a form alter in comment.module
3. Move "Shortcut icon" to /admin/config/system/site-information

Then we can remove global theme settings.

Is this still relevant? There has not been an update in over 10 years.

I did some digging to see what this is about. There is a system.admin_compact_page route that sets the admin_compact_mode cookie. There is also a system_compact_link render element to generate a link to toggle this cookie. This render element is currently used on "administrative index page templates" via template_preprocess_admin_page and "admin index templates" via template_preprocess_system_admin_index. Example URLs are /admin/index and /admin/config.

The issue summary mentions #30984: Usability: Provide descriptions for permissions → , but it seems we're already using the same link in \Drupal\user\Form\UserPermissionsForm::buildForm. It also mentions #205226: Use system_admin_compact_mode to hide $help text → which has not been done. Perhaps this should be converted to a meta and the two issues mentioned should be child issues.

From my POV I've never used the link or noticed that exists, so I don't have strong feelings towards this. My expectation would be that users have a preferences page linked to their account, rather than a cookie, and they could toggle on descriptions and help text from there.

Changing the status to PMNMI until we know if this is something we want to follow up.

While it's probably quite disruptive, I kind of agree. All the other links are nouns, but this is a verb. Help is both, but I read it as a noun, i.e. this is the page where you will find help (noun). Opened an MR, added the issue template and added screenshots.

mstrelan → made their first commit to this issue’s fork.

Is this issue still relevant? There has been no update in over 10 years and it is currently the oldest open issue in the system.module component. On that note, it's not really related to the system module, I suspect it was used as a catch-all. The current tabledrag library is defined in core.libraries.yml, outside of any modules or themes, so I think it's appropriate to move this to the javascript component.

I tried to understand the intent of the issue, but it's not really clear from the issue summary. There is mention of a prototype, but the link is broken. If anyone wants to continue this issue I think we need to start with clear direction in the issue summary. Changing status to Postponed (maintainer needs more info).

Not sure how to reproduce this one, but if the failing part doesn't exist then it can't fail, right?

I had a go at using the setupFile arg to provide the setup required for the test. Before doing this, the nightwatch test would complete in around 26 seconds, performing 11 HTTP requests before getting to the login step. Once I moved the module and permission setup to a setup file this was reduced to around 16 seconds, with 1 HTTP request before the login. But to use ::createUser I had to borrow a few assert methods from phpunit, and I figured we could just assign the perms to anonymous user. This eliminated all initial HTTP requests. I then noticed there were several permissions that were not required for the test to pass, so I removed those as well.

I guess this should be postponed on ✨ Provide PHP helpers for Nightwatch tests to speed up routine operations Active , but if it helps with the current random fails maybe we can keep it as it is until we have some generic helpers.

mstrelan → created an issue.

mstrelan → changed the visibility of the branch 3521621-dialog-deprecations-timeout to hidden.

Added 🐛 [random test failure] jQuery Events Deprecation Tests (Tests/dialogDeprecations) Active

As a bonus we can use REPEAT_TEST_CLASS now that it's a phpunit test.

Original (100ms timeout) failed 46/100 times - https://git.drupalcode.org/issue/drupal-3521621/-/jobs/5093110/viewer
Updated (wait for element) failed 0/100 times - https://git.drupalcode.org/issue/drupal-3521621/-/jobs/5092934

So we could just apply the same fix to the nightwatch test, but I think the 5x speed boost makes phpunit worth it.

Most likely this is because we're not waiting for .ui-dialog-content to exist, we're just waiting an arbitrary 100ms. But also it takes 30 seconds for me to run the nightwatch test, compared to 6 seconds to run the same in phpunit. This is likely due to all the setup required for nightwatch that happens via the UI. For this reason I've converted to phpunit.

mstrelan → created an issue.

According to the page linked in #34 we also need to add a dependency evaluation to the issue summary. I've inserted the headings for this. Considering this is only an interface and provided by php-fig I'm not sure it's all that relevant. Have added a bit of info to the headings.

mstrelan → changed the visibility of the branch 3497701-multirun-as-is to hidden.

This is not really in the scope of this module. You could perhaps create the new state and use a module like scheduled_transitions to move to that state. Then you could configure an email for that state change.

Rebased and managed to get a test that fails before and passes after, can see in the test-only changes job. Took me a while to figure out, especially since I was setting 'foo' => 'bar' in settings and changing it to 'foo' => 'baz', but the latter was overridden by \Drupal\common_test\Hook\CommonTestHooks::jsSettingsAlter.

If it's preferred we can bundle this in to 🐛 [random test failure] PackageUpdateTest::testPackageUpdate Active and commit in one place rather than having two separate issues.

mstrelan → changed the visibility of the branch 3497701-random-test-failure to hidden.

That's fantastic, thanks @znerol. I noticed 🐛 [random test failure] PackageUpdateTest::testPackageUpdate Active was not in the issue summary so I've added that. I didn't find any open issues for the other package manager build tests on the dashboard, but hopefully it's the same fix. We can see if they drop off the dashboard once the other is committed.

Upstream issue is fixed in composer-stager 2.0.1, opened 📌 Bump php-tuf/composer-stager to 2.0.1 Active .

mstrelan → created an issue.

Does this need a change record?

#13 - the proposal is not to change all instances everywhere, it is to allow either. Issues to switch from one to another should be closed won't fix unless the maintainers actively want to switch.

Currently someone adding image styles would already need to add the convert action, so I guess this setting would just provide the default format at that point? Or would we look to at the conversion to all image styles? Also, not all image styles have dimensions so I guess the source filesize could be useful there

One more set of tests:

No fix with 1/500 fails - https://git.drupalcode.org/issue/drupal-3519621/-/jobs/4992774
Fix branch with 0/1000 fails - https://git.drupalcode.org/issue/drupal-3519621/-/jobs/4992773

Another run with a fail before the runner died https://git.drupalcode.org/issue/drupal-3519621/-/jobs/4992772

And a run with the fix and 0 fails https://git.drupalcode.org/issue/drupal-3519621/-/jobs/4992730

This is a pretty simple one. In the test we load a page with an iframe and immediately start making assertions about it. But the content of the iframe takes some time to load. Most of the time it's very quick, but if we inject some artificial slowness in to \Drupal\media\Controller\OEmbedIframeController::render then we can reliably reproduce it. On my local it is sufficient to add usleep(50000);.

Here's the failing job (2/500) - https://git.drupalcode.org/issue/drupal-3519621/-/jobs/4992589

Pushing up a fix

Opened 🐛 [random test failure] MediaSourceOEmbedVideoTest::testMediaOEmbedVideoSource Active as I ran in to it yesterday and it was previously reported in #188.

mstrelan → created an issue.

Submitted a review but it seems to be the same stuff @longwave already suggested and was apparently fixed, so maybe a rebase went wrong?

Yes but we're talking about whether to convert to AVIF or webp, not whether to run the image style at all.

Right, same thing. In most cases we already know the dimensions of the output. So if the image style is for a small thumbnail, just enable the convert to webp plugin. If it's for a hero image then enable the convert with fallback plugin (or just convert to avif if you know its supported). Point is, the source image could be very large or very small but the output dimensions are the same.

I've attached an example in a zip since we can't upload webp or avif here. Original image is 7.4 MB. When I upload it to the media library we get a thumbnail by default. As a WEBP that's 3.3 KB but if I convert to AVIF it's 2.6 KB. That would fall under the "not worth the CPU cycles" threshold, but since the original is 7.4 MB we convert it anyway.

Some more conversions at 📌 [PP-1] Convert functional tests in navigation module to kernel tests Postponed .

I was able to work around more issues with multiple drupalGet calls, this time with lazy builders. The issue was that we needed to reset the MemoryBackend cache after the first drupalGet. This is something we already do in UiHelperTest by calling $this->refreshVariables. Here is the commit in that issue in case we want to add it here.

But I ran in to another issue - installing modules doesn't reset the hook implementations. So in NavigationDefaultBlockDefinitionTest::testNavigationDefaultBeforeNavigation where it tries to install navigation_test_block first, and navigation later, the hook_page_top implementation in navigation module is never invoked. I'm not sure a way to solve that yet.

I don't think we can generate two outputs to compare and pick one to use. The URL to the file is generated before the file is generated and as catch pointed out they would have different file extensions.

I would have though the threshold would be based on the file size of the original image (e.g. what @andypost says in #6), so wondering why you think it would be useless?

If your image style is a 16x16 pixel thumbnail and your original image is 100MB the JPG thumbnail will still be very small.

I would suggest adding to the new AvifConvert plugin. What is the logic for the threshold, is it the original file size? File size after converting (tricky)? Dimensions? The first one is fairly useless, the second one I think impossible since the derivative won't exist first time, and third one you can just opt not to use the convert plugin on the image style.

mstrelan → created an issue.

I don't think \Drupal\Tests\Composer\Plugin\Scaffold\Fixtures::createIsolatedComposerCacheDir actually works. It creates the dir and sets the env var, but composer doesn't actually use it.

I verified this by putting a breakpoint after the composer require command and inspecting the tmp dir and seeing it's empty. Then I added the following command before composer require and when I step through it the cache gets populated.

$this->mustExec("composer config cache-dir " . getenv('COMPOSER_CACHE_DIR'), $sut);.

So if we want to pre-populate the cache then we need to ensure the cache dir actually works.

You're right, it did, see https://git.drupalcode.org/issue/drupal-3354138/-/jobs/4978417/viewer

I suppose we could run it a few more times to see if it's consistently in that realm, but it's pretty slow (48 mins for 500 runs). Then we'd need to run the fix that many times as well.

Had a look in to this and not sure it is entirely accurate anymore. I started by disabling wifi and running ScaffoldTest, yet it passed every time. Possibly there is a composer cache somewhere, but it passed even after clearing my local composer cache. Regardless, the test apparently creates an isolated cache for each test run.

Then I tried ScaffoldUpgradeTest, but it is currently skipped because 8.8.0 doesn't support Composer 2. If I unskip it and bump the requirement to 9.5.0 it then complains that the path repo with 11.x-dev has higher priority. I've managed to work around that and have opened 📌 Unskip ScaffoldUpgradeTest Active to fix it. With wifi disabled I then get "Could not resolve host: repo.packagist.org", and once I enable wifi the test passes.

So we need to verify that ScaffoldTest is indeed accessing packagist (maybe the path repo is taking priority now) and we need a solution for ScaffoldUpgradeTest when repo.packagist.org is down. Maybe we can set up a mock mirror or we can prepopulate the cache from a fixture.