Issues for Drupal core

Remove the ability to update modules and themes via authorize.php

catch — Wed, 04 Dec 2024 18:55:17 +0000

Problem/Motivation

See discussion in #3253158: Add Alpha level Experimental Update Manager module and #3483501: Rename update module back to Update Status. I have been sure we already had an issue for this but haven't been able to find it despite at least a couple of attempts, so opening a new issue.

With package_manager, automatic updates (update manager) and project browser coming into core, we need to completely remove this functionality so it's not implemented twice in two different ways.

Steps to reproduce

Proposed resolution

The 1st two are 11.2.0 release priorities, and each will have parts of the MR changes from here. The last one is a 12.0.0 task to finally remove everything.

Remaining tasks

~~Product manager review to decide if we can remove outright (see MR !10461), or need to do some horrible deprecation dance with all this code.~~
Decide what to do with the administer software updates permission. It's also used for update.php (DB updates), so I think it needs to stay, but TBD.
Decide if we can remove lib/Drupal/Core/Archiver/* and related tests, too.
~~Decide if we can outright delete all the things we deprecated in #3417136: Remove adding an extension via a URL, or if we should be more careful.~~

User interface changes

See child issues.

API changes

See child issues.

Data model changes

Release notes snippet

[12.x] Remove all legacy code related to authorize.php and FileTransfer

dww — Wed, 29 Jan 2025 02:19:00 +0000

Problem/Motivation

Split from #3491731: Remove the ability to update modules and themes via authorize.php

This issue is for 12.x to (finally!) remove authorize.php, the FileTransfer classes, and the rest of the Updater classes.

Note that "update.module" is slightly a lie for component, since it's technically all code living in "base system", but it only exists for the legacy update manager and it's effectively part of update.module.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Deprecate authorize.php and the FileTransfer system

dww — Wed, 29 Jan 2025 02:18:56 +0000

Problem/Motivation

Split from #3491731: Remove the ability to update modules and themes via authorize.php

This issue is for 11.2.x to deprecate authorize.php, the FileTransfer classes, and the rest of the Updater classes.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

All of these are now deprecated:

core/authorize.php
system_authorized_init()
system_authorized_get_url()
system_authorized_batch_processing_url()
system_authorized_run()
system_authorized_batch_process()
core/lib/Drupal/Core/FileTransfer/*
core/lib/Drupal/Core/Updater/*
drupal_get_filetransfer_info()
hook_filetransfer_info()
hook_filetransfer_info_alter()
drupal_get_updaters()
hook_updater_info()
hook_updater_info_alter()
hook_verify_update_archive()

Data model changes

Release notes snippet

Remove UI and routes for the ability to update modules and themes via update.module and authorize.php

dww — Wed, 29 Jan 2025 02:18:47 +0000

Problem/Motivation

Split from #3491731: Remove the ability to update modules and themes via authorize.php

This issue is for 11.2.x to remove the UI and routes related to the legacy "Update manager" parts of update.module that allow updating your existing contrib modules and themes via the core UI.

Steps to reproduce

Proposed resolution

Remove the routes
Remove the form(s)
Remove links / local actions
Remove mentions in help text or other UI text

Remaining tasks

User interface changes

The legacy "Update Manager" parts of update.module are all gone. These routes no longer exist:

update.report_update (/admin/reports/updates/update)
update.module_update (/admin/modules/update)
update.theme_update (/admin/appearance/update)
update.confirmation_page (/admin/update/ready)

These forms are now gone:

core/modules/update/src/Form/UpdateManagerUpdate.php
core/modules/update/src/Form/UpdateReady.php

Introduced terminology

API changes

Data model changes

Release notes snippet

Add a drupalGet() method to KernelTestBase

joachim — Wed, 27 Sep 2023 15:11:33 +0000

Problem/Motivation

Many browser tests only need to check something in the returned content of a controller. This can actually be done in a Kernel test if the functionality provided by a new drupalGet() method is utilised.

Kernel tests are significantly faster than browser tests (see #3041700: [META] Convert some tests into Kernel or Unit tests).

Therefore by creating this new method it will allow many existing tests to be converted to kernel tests thus speeding up the pipeline as a whole.

Steps to reproduce

Use a drupalGet() method inside a Kernel test. It will not work because no KernelTestBase::drupalGet() method is currently defined.

Proposed resolution

Add a trait for kernel tests which provides a drupalGet() method. This new version of the existing drupalGet() method BrowserTestBase::drupalGet() will perform the same tasks the current version does but in Kernel tests. The method should output HTML files for debugging in the same way that BrowserTestBase::drupalGet() does.

Code added to KernelTestBase should avoid duplicating code in BrowserTestBase -- this may require code being refactored from UiHelperTrait to a new trait.

Also to consider (for naming of traits, refactoring choices etc) is that as a follow-up, submitForm() should be made available to KernelTestBase as well.

Remaining tasks

User interface changes

None

API changes

A new trait for kernel tests, HttpKernelUiHelperTrait, which allows kernel tests to make HTTP requests to the test site, and make assertions against the returned content with Mink.

Data model changes

None

Release notes snippet

A new trait for kernel tests, HttpKernelUiHelperTrait, allows kernel tests to make HTTP requests to the test site and make assertions against the returned content with Mink. This has the potential for many browser test to be converted to kernel tests, which are much faster to run because unlike browser tests, they don't set up a test site by running the Drupal installer through browser requests.

Notes from original report

Working-but-in-need-of-clean-up code is here:

- Add code from https://github.com/mglaman/drupal-test-helpers/tree/main/src to KernelTestBase
- https://github.com/mglaman/drupal-test-helpers/commit/2dbed403a1872a3aea...
- https://git.drupalcode.org/project/layout_paragraphs/-/merge_requests/12...

Improve FileValidator help

vladimiraus — Tue, 28 Jan 2025 12:47:25 +0000

Problem/Motivation

Improve FileValidator help

Remaining tasks

Add help
Update guide

Steps to reproduce

Proposed resolution

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Disallow AI bots by default in robots.txt

kevinquillen — Wed, 01 Nov 2023 17:22:25 +0000

Problem/Motivation

Users should be protected from AI bot(s) scraping by default. If they want to allow it, they can choose to do so after the fact by editing robots.txt or using modules like RobotsTxt. This would protect users and teams either unaware of AI bot scraping, those who don't want it, and or those who are not aware they need to be taking action (in this manner).

Web pages crawled with the GPTBot user agent may potentially be used to improve future models and are filtered to remove sources that require paywall access, are known to primarily aggregate personally identifiable information (PII), or have text that violates our policies. Allowing GPTBot to access your site can help AI models become more accurate and improve their general capabilities and safety.

I think this is a sensible change and also doesn't put full trust into ChatGPT or Google Bard not ingesting things or interpreting sensitive content as not-sensitive.

Proposed resolution

Add the following to the default robots.txt to block Google and OpenAI (ChatGPT):

User-agent: GPTBot
Disallow: /

User-agent: CCbot
Disallow: /

User-agent: anthropic-ai
Disallow: /

User-agent: Claude-Web
Disallow: /

User-agent: Google-Extended
Disallow: /

It may also be relevant to add an ai.txt by default for a broader disallow (is this a real growing standard or proposed?).

Deprecate HandlerBase defineExtraOptions

smustgrave — Fri, 01 Nov 2024 14:12:09 +0000

Problem/Motivation

It was discovered in #3478172: Add MissingParamType for views that in core/modules/views/src/Plugin/views/HandlerBase.php the method defineExtraOptions() isn't used.

Searching contrib space https://git.drupalcode.org/search?group_id=2&page=2&scope=blobs&search=d... I don't think anyone is using it. So it is probably safe to deprecate it.

Steps to reproduce

N/A

Proposed resolution

Deprecate the function

Remaining tasks

Deprecate
Deprecation test coverage
Change record

User interface changes

N/A

Introduced terminology

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Add a fallback classloader that can handle missing traits for attribute discovery

catch — Tue, 28 Jan 2025 19:18:30 +0000

Problem/Motivation

See #3490322: Use nickic/php-parser for attribute-based plugin discovery to avoid errors for some discussion. This doesn't replace that issue, it just provides a possible alternative way to handle missing traits, which is part of what that issue deals with.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Change $previous argument for HttpException to \Throwable

znerol — Tue, 21 Jan 2025 15:43:51 +0000

Problem/Motivation

Upstream bug report: https://github.com/symfony/symfony/issues/30728
Upstream PR: https://github.com/symfony/symfony/pull/30729

Steps to reproduce

Proposed resolution

Change $previous argument for every child of Symfony HttpException to \Throwable

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Use nickic/php-parser for attribute-based plugin discovery to avoid errors

godotislate — Wed, 27 Nov 2024 21:08:38 +0000

Problem/Motivation

Using the nikic/php-parser library has been mentioned is a possible alternative to parsing attributes via PHP reflection, because substantive use of reflection can result in performance and memory usage concerns.

Using nikic/php-parser to retrieve attribute values will retrieve some or all attribute values in the case of missing dependencies.

From comment #12 on #3395260: Investigate possibilities to parse attributes without reflection

A problem that's come up in #3421014: [PP-1] Convert MigrateSource plugin discovery to attributes and #3458177: Changing plugins from annotations to attributes in contrib leads to error if plugin extends from a missing dependency is that attributes can't be read via Reflection on classes that implement missing interfaces, extend missing classes, or use missing traits, in the case that the missing dependency comes from an uninstalled module. While Reflection will throw an exception for a missing interface or class, PHP will fatal on a missing trait. The exception thrown during plugin discovery was handled in 3458177, but a solution for handling missing traits gracefully is still outstanding.

Steps to reproduce

For example, the migrate source plugin Drupal\block_content\Plugin\migrate\source\d7\BlockCustomTranslation in the Block Content module uses Drupal\content_translation\Plugin\migrate\source\I18nQueryTrait from the Content Translation module. Block Content does not have a module dependency on Content Translation, so it migrate source plugins were using attributes, discovery would lead to a PHP fatal error when Migrate and Block Content were installed, but not Content Translation.

Proposed resolution
Move nikic/php-parser from a dev composer requirement to runtime

Use nikic/php-parser on plugin classes during discovery before reflection to identify interface, class, and trait dependencies the class has that may be missing

As an additional measure, provide a Dependencies attribute that can be applied to classes. Its only property is modules, which is a list of module machine names that need to be installed in order for the plugin to be discovered

Use nikic/php-parser to check whether the plugin class has the attribute and to extract the list of module dependencies

Avoid reflection on classes with missing dependencies and do not save to the discovery file cache
Developers can then manually declare on their plugin classes which modules need to be installed for the plugin to be discovered like so:
#[MigrateSource(
  id: 'd7_block_custom_translation',
  ...
]
#[Dependencies(['content_translation', 'migrate_drupal'])]
to ensure that discovery ignores the class and does not fatal error.

Specifying the dependencies likely should be unnecessary, since the missing trait should have already been identified, but complex cases where a missing trait is somewhere higher in the class hierarchy would not be handled without the Dependencies attribute.

Note that the nikic parser is not used here to replace Reflection for attribute-based plugin discovery. For now, it is only making attribute-based plugin discovery more error-resistant.

This approach also provides a start on how attribute parsing via nikic can be done. Dependencies is a simple attribute with only one array property. Plugin attribute classes can be a lot more complex, with nested object properties, so that will need further investigation in #3395260: Investigate possibilities to parse attributes without reflection .
Remaining tasks

Make a decision per #8:

So yes, someone needs to make a decision whether

No class/trait/interface checks. Require explicit dependencies and maintain less core code.

Middle ground: immediate classes checked with direct class_exists & friends

Whole hog. Make DrupalKernel::$classLoader public readonly, use $kernel->classLoader->findFile() to find any classes/traits/interfaces being depended on, read 'em, parse 'em and recurse with a static cache.

I have no opinion which one is best, I merely outlined how #3 could look.

And/or use statements could easy be parsed for non-existent modules.

The first two likely need nikic/php-parser. The third probably doesn't, because the existing Doctrine-based StaticReflectionParser from core and migrate can do this.

Also, the solution used here for plugin discovery could possibly be re-used on all Reflection-based discovery as it eventually expands to routes and services.

For plugin subsystem manager and framework manager review:

OK to use nikic/php-parser library

Decision presented above (per #8) about which strategy to take

Whether any of this can apply to other existing/future discovery by attributes for hooks, services, routes, and if anything should be done here to make more reusable

Whether to refactor StaticReflectionParser to use nikic/php-parser in a followup

User interface changes

N/A

Introduced terminology

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Review cache bin and cache tags of access policy caching

berdir — Sat, 18 Jan 2025 22:09:00 +0000

Problem/Motivation

This is about two caching things in AccessPolicyProcessor.

See #3436146-9: Introduce a list of "common cache tags" to reduce lookup query amount. The access policy includes dynamic cache tags for user roles very early in bootstrap.

Those can't be easily preloaded and preloading them all requires more complex preloading logic.

The question is whether it's worth to add cache tags for specific roles there. Roles typically don't change often, it might be worth to just use the list cache tag. This is relatively minor and might actually mostly go away with the ideas below for just core at least.

The second part is that it uses a separate, new cache bin just for itself. For the database backend, this doesn't make a big difference.

But for redis/memcache, each unique bin is a performance cost, because redis/memcache have no means to delete/invalidate across a whole bin. Redis maintains both a separate last-delete-all timestamp per bin and a cache tag for invalidateAll (See #3498947: Deprecate CacheBackendInterface::invalidateAll() and #3500680: Allow to remove support for invalidateAll() and treat it as deleteAll().

Right now, the current access policy cache implementation therefore results in 5 operations based on the monitor output from #3498940: Optimize bin cache tags and last write timetamp:

"HGETALL" "core:access_policy:access_policies:drupal:[user.is_super_user]=1:[user.roles]=authenticated,administrator"
"GET" "core:cachetags:x-redis-bin:access_policy"
"GET" "core:access_policy:_redis_last_delete_all"
"HGETALL" "core:access_policy:access_policies:drupal:[languages:language_interface]=en:[user.is_super_user]=1:[user.roles]=authenticated,administrator"
"MGET" "core:cachetags:config:user.role.authenticated" "core:cachetags:config:user.role.administrator" "core:cachetags:access_policies"

Explanation:
1. first cache get, the redirect
2. this needs to check if there was an invalidateAll() on the bin, so it checks the cache tag for it. This could be skipped with the referenced invalidateAll() issues.
3. then it needs to check if there was a deleteAll() on the bin
4. then the real cache get with the redirected cache key
5. then it checks the 3 specific cache tags in my case for admin.

All of this is purely done to support more complex implementations that the majority of sites are actually not using. For the implementation in core, the only thing this is caching is the roles, which are in the ChainedFast config bin.

This is a lot of lookups for what it's doing, so setting to major.

Steps to reproduce

Proposed resolution

I'm not sure. For core, it could use discovery bin, but I understand that sites using group module for example might have a lot of different cache variations. But we IMHO need a way to avoid all this overhead for the vast majority of sites that don't have expensive access policies.

What if we add some kind of interface or method to indicate whether access policies are actually worth to cache? Group can do that, Core won't, and then we just use the static cache if none of the access policy services we have require persistent cache?

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CSRF token generation incompatible with optional route parameters

bradjones1 — Tue, 24 Jul 2018 23:57:27 +0000

Problem/Motivation

RouteProcessorCsrf::processOutbound() does not take into account optional parameters when calculating a CSRF token, leading on-request validation to fail on a generated route in which one or more parameters were not provided at the time of generation.

Steps to reproduce

Create a new custom module with a controller. You can use drush generate module and drush generate controller to ease the generation of files.

Reconfigure module routing as follows:

my_module.routing.yml

my_module.example:
  path: '/my-module/example'
  defaults:
    _title: 'Example'
    _controller: '\Drupal\my_module\Controller\MyModuleController::example'
  requirements:
    _permission: 'access content'
  options:
    no_cache: TRUE

my_module.protected:
  path: '/my-module/protected/{param1}'
  defaults:
    _title: 'CSRF Protected page'
    _controller: '\Drupal\my_module\Controller\MyModuleController::protected'
    param1: ''
  requirements:
    _permission: 'access content'
    _csrf_token: 'TRUE'

MyModuleController.php

final class MyModuleController extends ControllerBase {

  public function example(): array {

    $url1 = Url::fromRoute('my_module.protected', ['param1' => 'value']);// --> /my-module/protected/value <-- Works.
    $url2 = Url::fromRoute('my_module.protected', ['param1' => '']);// --> /my-module/protected <-- Works.
    $url3 = Url::fromRoute('my_module.protected', ['param1' => NULL]); // --> /my-module/protected <-- Works.
    $url4 = Url::fromRoute('my_module.protected', []); // --> /my-module/protected <-- Will NOT work.

    $build['link1'] = [
      '#type' => 'link',
      '#title' => $this->t('Link 1'),
      '#url' => $url1,
    ];
    $build['link2'] = [
      '#type' => 'link',
      '#title' => $this->t('Link 2'),
      '#url' => $url2,
    ];
    $build['link3'] = [
      '#type' => 'link',
      '#title' => $this->t('Link 3'),
      '#url' => $url3,
    ];
    $build['link4'] = [
      '#type' => 'link',
      '#title' => $this->t('Link 4'),
      '#url' => $url4,
    ];

    return $build;
  }

  public function protected(): array {

    $build['content'] = [
      '#type' => 'item',
      '#markup' => $this->t('It works!'),
    ];

    return $build;
  }

}

Enable the module
Rebuild caches
Visit the page /my-module/example and click in all four links. The fourth link will not pass the CSRF validation

Proposed resolution

Unify the code incore/lib/Drupal/Core/Access/CsrfAccessCheck.php and core/lib/Drupal/Core/Access/RouteProcessorCsrf.php and take into account all route parameters for the generated token.

Remaining tasks

[✓] - Fix
[✓] - Test for new funcionality

User interface changes

CSRF protected generated links for routes with optional parameters now work as expected.

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

Original report by @bradjones1

"To log in to this site, your browser must accept cookies from the domain" error message displayed when user goes back and reload the page

carolpettirossi — Mon, 30 Oct 2023 14:30:58 +0000

Problem/Motivation

The message "To log in to this site, your browser must accept cookies from the domain" is displayed to the user when they use "Back" button and refresh the page.

Steps to reproduce

1. Login with any user
2. Log out
3. Click to go back (Back button of the browser)
4. Reload the page
5. See the message

The message is quite confusing for the user when they perform the steps above as it doesn't make sense.

Is this by design?

Proposed resolution

In short, the proposed resolutions is to set a new cookie and add a redirect to a new verify-cookies page when the check_logged_in query parameter is present but there is no session for the request. This new page will ensure cookies are actually disabled before displaying an error message and the user is not just accessing a url with the check_logged_in query parameter while logged out.

This solution was inspired by comments on the original issue (#2946: Login fails and no warning is issued if cookies are not enabled) which suggested a double redirect approach. Except in the proposed resolution the redirects only occur for users who have cookies disabled or who directly access a url with the check_logged_in query parameter when they are logged out. So it would rarely be experienced by regular users.

Details:

Keep the existing functionality that sets the check_logged_in query parameter but refactor Cookie::applies so that it instead sets a property on the class that can be checked by a new kernel request event listener
Add a new kernel request event listener that checks for the new \Drupal\user\Authentication\Provider\Cookie property, and if it is set to TRUE, sets a LocalRedirectResponse to a new user.verify_cookies route and sets a cookie that the new route will check for.
Add a new user.verify_cookies route and controller method in UserAuthenticationController
The new verify cookies controller method will check for the new cookie and if it doesn't exist will display an error message as cookies are most likely disabled. If the cookie is present it will redirect the user to their original request.

Sync language list with translations (add fy, ga, hy, km, ms and sw)

gábor hojtsy — Mon, 12 Aug 2013 09:04:18 +0000

Problem

We maintain a list of languages in core for which there are translations, so users get to pick a language that Drupal can actually install in. We last synced this list in #1295696: Sync predefined list of languages with localize.drupal.org and extend native language information and then removed languages where translations were not actually available in #1904214: Installer auto-selects languages that don't have a translation, causing a requirements error. Since several languages were added and got translations. We noticed Khmer and Bahasa Malaysia missing via CKEditor language mappings in #2050097: Map CKEditor languages to Drupal languages. Others were missing when comparing the online and shipped language list.

Proposal

Add the missing languages to core. The native names were taken off of localize.drupal.org directly. This is a very easy fix. I checked and there are no other languages that are missing but have translations on localize.drupal.org at this point (checked against Drupal 7.23 translations at http://ftp.drupal.org/files/translations/7.x/drupal/). Also checked and no languages need to be removed on the grounds of no translations being available.

Add test coverage for Field API Number

Anonymous — Mon, 24 Aug 2009 21:08:46 +0000

We've had some recent WSODs and critical validation errors on core Field API widgets:

#557932: Taxonomy term field autocomplete widgets never validate, always lose values.
#557924: Options widget broken

Both of these widgets have things in common: they both use custom multiple values handling. They also have something in common with other D7 core Fields: very little test coverage. The only widgets with any tests in core are text_textfield and text_textarea.

Allow plugin service wiring via constructor parameter attributes

longwave — Tue, 05 Jul 2022 10:11:09 +0000

Problem/Motivation

To wire services to a plugin, developers must implement ContainerFactoryPluginInterface and a custom create() method that pulls services from the container and calls the plugin constructor. This is largely boilerplate code and must be written for each plugin implementation.

Using PHP 8 attributes, we can tag constructor parameters directly and allow the factory class to discover and inject the required services, and drop the create() method entirely.

This also has the advantage of placing the service name directly next to the parameter where it is used, instead of having to keep the create() method and constructor signature in sync in two different places.

Steps to reproduce

Proposed resolution

Add support to ContainerFactory for Symfony's #[Autowire] attribute.

Remaining tasks

Determine if there are any more edge cases to cover.

User interface changes

API changes

Plugins can now wire services by specifying an #[Autowire] attribute on the each constructor parameter, instead of writing a separate create() method.

Data model changes

Release notes snippet

Inappropriate error when updating a field schema with existing data

usingsession — Tue, 28 Jan 2025 18:13:25 +0000

Problem/Motivation

When attempting to update a field's cardinality programmatically using the `EntityDefinitionUpdateManager`, an error occurs if the field is stored as a property in the entity table, but the update attempts to move the field to a dedicated field table.

The error message is as follows:

<strong>TypeError: array_intersect_key(): Argument #1 ($array) must be of type array, null given in array_intersect_key()</strong> (line 2479 of /core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php)

Steps to reproduce

Install a fresh Drupal core with the Node module enabled.
Create content using the "Node" entity type, and ensure the "Title" field has data stored as a property in the entity table.
Create a custom module with the following hook or equivalent programmatic update (the code below)
Run the update or execute the programmatic code to update the field storage definition.
Observe the error.

  <?php

   use Drupal\Core\Field\FieldStorageDefinitionInterface;

   /**
    * Implements hook_update_N().
    */
   function mymodule_update_9001() {
     // Update definitions and schema.
     /** @var \Drupal\Core\Entity\EntityDefinitionUpdateManager $manager */
     $manager = \Drupal::entityDefinitionUpdateManager();

     /** @var \Drupal\Core\Field\BaseFieldDefinition $definition */
     $definition = $manager->getFieldStorageDefinition('title', 'node');
     // Change cardinality to trigger the schema transition.
     $definition->setCardinality(FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED);
     $manager->updateFieldStorageDefinition($definition);
   }

Proposed resolution

Add a check in `SqlContentEntityStorageSchema::hasColumnChanges()` to identify schema transitions where a field moves from being stored as a property in the entity table to a dedicated field table.
This check will correctly detect schema transitions and allow the system to throw a meaningful exception instead of a generic error.
Ensure that when such a schema transition is detected, the system throws a `FieldStorageDefinitionUpdateForbiddenException` with a clear message, such as:
The SQL storage cannot change the schema for an existing field (title in node entity) with data.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Incorrect path used in a A11y Test Admin

the_g_bomb — Wed, 22 Jan 2025 15:52:32 +0000

Problem/Motivation

As highlighted in:
#2857808-26: Automate Accessibility Checks for Core
It looks like there is an error in the admin tests.

{ name: 'Create Article', path: '/user/1/edit' },

Perhaps it should be:

{ name: 'Create Article', path: '/node/add/article?destination=/admin/content' },

Steps to reproduce

Visit: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/...
See the Create article link is pointing to the incorrect URL.

Proposed resolution

Make the Create article test use the node add path: /node/add/article?destination=/admin/content

Remaining tasks

Fix it
Review

isMultilingual() loads all languages just to check if there is more than one

berdir — Sat, 12 Mar 2016 01:40:23 +0000

Problem/Motivation

For fun, I'm currently trying to get rid of as many queries as I can on a simple site. And i noticed that isMultiple() loads all languages just to count them.

Proposed resolution

LanguageServiceProvider could store a parameter or something in the container that we can then access in ConfigurableLanguageProvider

Remaining tasks

User interface changes

API changes

Data model changes

Taxonomy Term List page is broken when users don't have permission to edit/create terms

carolpettirossi — Mon, 13 Jan 2025 16:40:11 +0000

Problem/Motivation

When user has access to view the list of terms/taxonomy overview page and they don't have access to manage them, the style is broken and confusing.
The user sees a + SVG but no indentation. It's quite hard to distinguish between parent and child terms.

Steps to reproduce

1. Create a taxonomy
2. Add few terms
3. Add children terms so you can get the indentation

Proposed resolution

Adjust the css/styling of the page

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Add stream wrappers to access extension files

dave reid — Thu, 13 Oct 2011 03:31:46 +0000

Problem/Motivation

Starting from #2351919: Replace uses of drupal_get_path() with __DIR__ where possible, when the PHP code needs to include or parse files inside the module or theme directory space, will simply use _DIR_ instead of drupal_get_path(). This is sufficient, intuitive and more performant than calling drupal_get_path().

However, drupal_get_path() is widely used to refer files (.js, .css, images, assets) outside a module or theme. This isn't very intuitive for new developers. Stream wrappers make much more sense and simplify the API for developers trying to refer directories or files from outside the current extension in code.

Proposed resolution

Introduce extension stream wrappers:

Scheme	Description
`profile://`	Points to the installed profile root directory.
`module://{name}`	Points to the module `{name}` root directory. Only enabled modules can be referred.
`theme://{name}`	Points to the theme `{name}` root directory. Only installed themes can be referred.

Examples

Profile: profile://

Assuming the standard profile is installed:

Referring a directory:
- Actual: drupal_get_path('profile', 'standard') . '/config'
- Proposed: profile://config
- Path: core/profiles/standard/config
Referring a file:
- Actual: drupal_get_path('profile', 'standard') . '/config/install/automated_cron.settings.yml'
- Proposed: profile://config/install/automated_cron.settings.yml
- Path: core/profiles/standard/config/install/automated_cron.settings.yml

Module: module://

Assuming the node module is enabled but color module is not:

Referring a directory:
- Actual: drupal_get_path('module', 'node') . '/config'
- Proposed: module://node/config
- Path: core/modules/node/config
Referring a file:
- Actual: drupal_get_path('module', 'node') . '/config/install/node.settings.yml'
- Proposed: module://node/config/install/node.settings.yml
- Path: core/modules/node/config/install/node.settings.yml
Referring a resource in a uninstalled or inexistent module:
- Actual: drupal_get_path('module', 'color') . '/config'
- Proposed: module://color/config
- Path: Throws \RuntimeException

Theme: theme://

Assuming the bartik theme is installed but seven theme is not:

Referring a directory
- Actual: drupal_get_path('theme', 'bartik') . '/config'
- Proposed: theme://bartik/config
- Path: core/themes/bartik/config
Referring a file
- Actual: drupal_get_path('theme', 'bartik') . '/color/color.inc'
- Proposed: theme://bartik/color/color.inc
- Path: core/themes/bartik/color/color.inc
Referring a resource in a uninstalled or inexistent theme:
- Actual: drupal_get_path('theme', 'seven') . '/config'
- Proposed: theme://seven/config
- Path: Throws \RuntimeException

Remaining tasks

Profiling to compare performance of the new code with the old.

API changes

New abstract class for extensions stream wrappers:

\Drupal\Core\StreamWrapper\ExtensionStreamBase

New stream wrapper classes:

\Drupal\Core\StreamWrapper\ModuleStream
\Drupal\Core\StreamWrapper\ThemeStream
\Drupal\Core\StreamWrapper\ProfileStream

Data model changes

None.

Some of comparison with Node.nodeType uses magic number

tom konda — Tue, 28 Jan 2025 01:48:01 +0000

Problem/Motivation

For example, in /misc/position.js, all of comparison against Node.nodeType use number value. The Node interface has already defined constant value which identifies nodeType so these values can compare with constant value.

Proposed resolution

Replace all of comparison against Node.nodeType use magic number with constant which is defined in the Node interface.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Revision revert links in workspace respond with 404 not found

malcomio — Thu, 11 Apr 2024 16:00:14 +0000

Problem/Motivation

When viewing revisions inside a workspace, the view includes links that respond with 404 page not found.

Steps to reproduce

1. create a node
2. create a workspace
3. switch to the workspace and create a new revision of a node
4. view revisions - the revision in the workspace is shown as the current revision
5. click on revert for the other revision (i.e. the current live revision)
6. this page responds with 404

Proposed resolution

tbc - prevent the option of reverting to the current live revision?
When reverting to other (non-current) revisions, a warning appears: "Form should not be submitted in a workspace"

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

handling upload limits from php.ini graciously ?

thony — Tue, 28 Jan 2025 11:08:08 +0000

Problem/Motivation

If you try to upload a file above Drupal limitation but within php.ini POST Content-Length, you l will get an informative error handled by Drupal, like 'the file you are trying to upload exceeds the limit [...]'

If you go above POST Content-Length the ajax call return a warning and you get the rather vague message :'Oops, something went wrong. Check your browser's developer console for more details.'

Of course, you can increase php.ini limits, but you might have other project constraints that prevent that.

Proposed resolution

Would it be possible for Drupal, to catch the error and display an error message in the same informative way ?

Update manager routes are not disabled anymore when allow_authorize_operations is FALSE

prudloff — Tue, 28 Jan 2025 14:49:54 +0000

Problem/Motivation

Our website using Drupal 10.3.5 has $settings['allow_authorize_operations'] = FALSE; but we recently noticed that our devs could access /admin/reports/updates/update again.

(This was discussed privately with the security team and it was decided it could be handled publicly.)

Steps to reproduce

You can see this vulnerability by:
1. Adding $settings['allow_authorize_operations'] = FALSE; in settings.php
2. Empty cache
4. As a user with "administer software updates" permission browse to /admin/reports/updates/update

Proposed resolution

I think it comes from this change: https://git.drupalcode.org/project/drupal/-/commit/206a3ac2a295e6ac21a6c...
The update.route_subscriber service does not have the event_subscriber tag so it is never called.
If I add it, the routes are correctly protected again:

  update.route_subscriber:
    class: Drupal\update\Routing\UpdateRouteSubscriber
    arguments: ['@settings']
    tags:
      - { name: event_subscriber }

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Allow starterkit theme generator tool to clone Olivero

mherchel — Mon, 01 Aug 2022 19:19:25 +0000

Drupal 10's new theme generator tool is intended to replace the need for creating subthemes from existing core themes. This will enable core themes to change markup without the need to take into account subthemes and backwards compatibility.

This issue is to allow the theme generator tool to clone Olivero.

Things that we'll need to do:

~~Bring build step into Olivero - this is the ability to compile PostCSS~~
~~Rename Olivero filenames.~~
~~Rename functions etc~~

File names that will need to be renamed

~~All of the config items within core/themes/olivero/config~~
~~core/themes/olivero/olivero.breakpoints.yml~~
~~core/themes/olivero/olivero.info.yml~~
~~core/themes/olivero/olivero.libraries.yml~~
~~core/themes/olivero/olivero.theme~~
~~core/themes/olivero/src/OliveroPreRender.php~~

File contents that will need to be renamed

~~Various config within /config~~
~~Lots of comments within the JavaScript~~
~~The JavaScript config that gets loaded at Drupal.olivero~~
~~Color settings that get loaded under drupalSettings.olivero~~
~~Various once names (these may be able to have the namespace removed)~~
~~Various CSS selectors (.olivero-details)~~
~~Various CSS Animation names (eg olivero-throbber)~~
~~References to Olivero in /templates (eg {% include '@olivero/includes/preload.twig' with { olivero_path: olivero_path } only %})~~
~~Comments within /templates~~
~~Libraries attachments ({{ attach_library('olivero/layout-views-grid') }})~~
~~Everything olivero related in olivero.theme and theme-settings.php~~

To figure out

Tests. This all seems pretty brittle. If we change something, we need to make sure the theme generator works.
We need to ensure that the user-supplied theme name is valid. This needs to work in the JavaScript, preprocessing, templates, etc.

Testing Instructions

Checkout the MR branch to local/gitpod
cd into the webroot
php core/scripts/drupal generate-theme <theme_machine_name> --name "<Theme Name>" --starterkit olivero
Check new theme for any files that have "olivero" in their name or file contents

Block visibility conditions with context not saved

mvonfrie — Tue, 28 Jan 2025 17:04:59 +0000

Problem/Motivation

The block visibility configuration is not saved for conditions having a context with multiple matching context providers when not the default (first) provider is selected.

This is related to the condition plugins because adding a context definition directly to a custom block type and selecting the non-default context provider works a expected.

Steps to reproduce

There are multiple conditions with a context available like group types, webforms and maybe even node types. I had the issue with group types so I will use this as example.

Install the group module and create two group types (I have multiple group types and do not know how the UI behaves with only one type).
Install the group_finder module to have a second group context provider available.
(optionally) Install the ctools module.
Go to configuration of any block.
1. For the "Group Types" condition select the "Group Finder provider.
2. Do not select any of the group types.
3. If you've installed ctools do the same for the "Group Type" condition (which is provided by ctools for all entity types).
4. Save the configuration.
(In a second window) go to admin/config/development/configuration/single/export and check whether the visibility condition has been exported correctly.
Go to configuration of the same block again.
1. For the "Group Types" condition select the "Group Finder provider.
2. Now select at least one of the group types.
3. If you've installed ctools do the same for the "Group Type" condition (which is provided by ctools for all entity types).
4. Save the configuration.
Repeat step 5.

In step 5 the context will not be exported, but in step 7 it will be exported correctly.

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Media Library item styles assume contextual module is present

bnjmnm — Tue, 28 Jan 2025 17:48:29 +0000

Problem/Motivation

For media item labels to be positioned correctly, the enclosing <article> needs to be position: relative;. If the page includes the contextual module libraries then the <article> will have the .contextual-region class and be styled by this bit in contextual.module.css

.contextual-region {
    position: relative;
}

There are legit scenarios such as this one with Experience Builder where the media library is used without contextual present -- even on sites with that module enabled.

It winds up looking like this, with the labels under the images out of place (not hugely so, but still not a great look)

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Add an optional site title to the Navigation

ckrina — Tue, 28 Jan 2025 18:56:41 +0000

Problem/Motivation

Some site might want to keep their Site name on the Navigation to differentiate from different Drupal sites.

Proposed resolution

Add the ability to show the Site name on the Navigation. That wouldn't be mandatory, it would be optional and disabled by default. The option to enable and disable the visibility should be handled on the same page as the Logo: /admin/config/user-interface/navigation/settings

On the Navigation, the site would be printed next to the logo taking a maximum of two lines. Site names longer than that will be trimmed.

Figma link.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Add a trait for autowiring properties in tests

mstrelan — Mon, 29 Jul 2024 02:23:03 +0000

Problem/Motivation

Currently when adding services as properties in tests you need to declare the properties and initialize them in ::setUp. This is unnecessary boilerplate.

Proposed resolution

Add a trait similar to AutowireTrait to initialize the services in setUp.
Add symfony/expression-language as a dev dependency. This allows adding autowiring based on expressions rather than pure service or parameter, like for example #[AutowireProperty(expression: "service('entity_type.manager').getStorage('node')")].
symfony/expression-language itself requires two more Symfony components through its dependency chain: symfony/cache and symfony/cache-contracts

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Compressed ajax_page_state['libraries'] can exist in both $request->request and $request->query simultaneously

tim.plunkett — Tue, 28 Jan 2025 19:06:03 +0000

Problem/Motivation

#3348789: Compress ajax_page_state added compression to the ajax_page_state['libraries'] and a middleware to uncompress it.
It first checks $request->request and uncompresses that.
If it wasn't found, it then checks $request->query and uncompresses that.

But what if it's in both? Then libraries are readded to the page. In the case of dialog.js, that leads to

Uncaught SyntaxError: Identifier 'DrupalDialogEvent' has already been declared (at dialog.js?v=11.2-dev:1:1)

Steps to reproduce

Add a modal dialog with an #ajax'd field at the same time
For example, in #3386762: Use modals in field creation and field edit flow.

Proposed resolution

Uncompress the libraries in both places

Remaining tasks

Write tests

User interface changes

N/A

Introduced terminology

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Integrate Umami message

ckrina — Mon, 15 Apr 2024 14:38:32 +0000

Problem/Motivation

The existing message provided by the Umami installation profile that shows on the Toolbar hasn't been integrated on the new navigation yet.

Proposed resolution

Create a new custom block navigation implementation that follows a design along this lines:

When collapsed, only the warning icon should be shown.

Remaining tasks

This will new to implement a custom block for the Navigation for the first time, beyond the existing menus.

User interface changes

The Umami message will appear on the new navigation.

Deprecate views_field_default_views_data and related functions

nicxvan — Sun, 24 Nov 2024 01:41:35 +0000

Problem/Motivation

It would be great to deprecate auto including .inc files defined by hook-hook-info groups.

In order to do that we first need remove helpers from provided groups.

This issue handles all of the functions related to views_field_default_views_data.

We replace the functionality, then deprecate the functions and move them to the .module files.

We use DI for the replacements only on a couple since views related dependencies need to be optional so they must be last.

The hooks using these services should be converted to DI in a followup.

Steps to reproduce

N/A

Proposed resolution

Move it to a service in the views module.
Replace calls.

Remaining tasks

Open follow up to add dependency injection to the hook classes using this. I didn't want to add this as the first injected dependency, but also didn't want to add DI to all the hook classes using this as part of this issue.

User interface changes

N/A

Introduced terminology

N/A

API changes

Deprecated functions:
views_field_default_views_data
datetime_type_field_views_data_helper
_views_field_get_entity_type_storage
_content_moderation_views_data_object

See cr for replacements.

Data model changes

N/A

Release notes snippet

Regression: RssResponseCdata filtering out common HTML tags from RSS feeds

wlofgren — Mon, 06 Jan 2025 22:22:57 +0000

Problem/Motivation

After upgrading to Drupal 10.4.0, I found that our RSS feed doesn't render div, img, and style tags. Doing so breaks our RSS email structurally (div) and visually (img and style).

Steps to reproduce

Proposed resolution

Remove filtering
Add XSS test

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Extend PhpUnitTestDiscoveryTest to test also PHPUnit API

mondrake — Sat, 04 Jan 2025 11:00:30 +0000

Problem/Motivation

In #3477634: Ensure run-tests.sh and PHPUnit CLI run with the same list of tests to be executed, we introduced a test checking equality between PHPUnit and TestDiscovery test discovery lists and fails if they differ.

We used a shell process to run the PHPUnit CLI for this purpose, and interpreted the output XML file.

PHPUnit also provides an API to retrieve the test list via PHP, avoiding the subprocess execution.

Using directly the API has a potential to eventually drop Drupal's own test discovery process.

Proposed resolution

For now, just add a check for the result of the API calls. Keep both CLI and API checks.

Dropping Drupal's TestDiscovery is left to a follow-up once we are confident we no longer need that.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Views HandlerBase::breakString works unexpectedly for a views with a huge amount of filters/parameters

Ihor Ukhan — Mon, 18 Dec 2023 08:37:41 +0000

Problem/Motivation

Views that generate requests with a huge amount of arguments (more than 3500) give WSOD.
The issue was reproduced on commerce order view with the amount of order items more than 3500.

Steps to reproduce

It was reproduced on commerce order view with a number of order items of more than 3500, so
1. Install commerce module
2. Create an order with the number of order items more than 3500.
3. Go to the order view page.

Expected result

Page (view) opened.

Actual result

The website encountered an unexpected error. Please try again later.
Error in the watchdog: TypeError: Cannot assign null to property Drupal\views\Plugin\views\argument\ArgumentPluginBase::$operator of type string in Drupal\views\Plugin\views\argument\NumericArgument->title() (line 63 of /usr/www/users/cbefce/web/core/modules/views/src/Plugin/views/argument/NumericArgument.php).

Root cause of issue

web/core/modules/views/src/Plugin/views/HandlerBase::breakString gives unexpected results in case of using a string with a huge amount of filter parameters, like "N1+N2+N3....+N3500" (it breaks somewhere at a number of filters ~ 3500).
The reason for that is preg_match can't handle a long string: For the longer string, the issue could be related to the length of the string causing a problem in PHP's regex engine. PHP has a limit on the size of the string it can process with regular expressions, and very long strings can lead to unexpected behavior, such as failing to match.

Proposed resolution

Fix web/core/modules/views/src/Plugin/views/HandlerBase::breakString to avoid using preg_match.

After selecting an image in media library remove icon is not visible

zeeshan_khan — Thu, 23 Jan 2025 14:25:17 +0000

Problem/Motivation

After selection of media/image in Media Library image close icon is hidden behind the image.

Steps to reproduce

Install Drupal 10.4
Set Claro as a default theme Or Admin theme
Create a custom content type or use 'Article' or 'Basic Page' built-in type
Create image field with media library and add the field to the content type
Create a node of the content type
Select an image in the media library field
Check that the 'image remove' option is not visible

Proposed resolution

In claro's "media-library.css" class = media-library-item__remove has z-index set to 1. We need to increase the z-index for remove icon to make it visible over the image.

Remaining tasks

User interface changes

Introduced terminology

None

API changes

None

Data model changes

None

Release notes snippet

SqlBase::prepareQuery() should be called also on count

claudiu.cristea — Mon, 05 Dec 2016 10:53:08 +0000

Problem/Motivation

Suppose that in a source plugin, extending from SqlBase, you want to override SqlBase::prepareQuery() and use that to add additional tags or meta-data to the query. You don't do that in ::query() method for some good reasons (for example you want to apply tags & meta-data only after the query was defined). If a hook_query_TAG_alter() that uses those tags adds a new condition then SqlBase::count() is lying.

For this reason, this is a blocker for #3069776: [PP-1]: SQL source plugins: allow defining conditions and join in migration yml, which introduces some query modifications in ::prepareQuery() method.

Proposed resolution

Make SqlBase::count() aware of SqlBase::prepareQuery().

Remaining tasks

Review / commit.

There are no User interface, API, or data model changes.

Block UI A11y errors identified with Axe + Nightwatch

bnjmnm — Mon, 31 Oct 2022 17:53:50 +0000

Blocked on #3293469: Automated A11y tests in Nightwatch, which performs the checks that surfaced this

Problem/Motivation

The tests added in #3293469: Automated A11y tests in Nightwatch identified the following when visiting /admin/structure/block

 ✖ NightwatchAssertError
08:56:50    aXe rule: color-contrast - Elements must have sufficient color contrast
08:56:50 	In element: .region-hero-message > td[colspan="5"] > em
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: color-contrast - Elements must have sufficient color contrast
08:56:50 	In element: .region-sidebar-message > td[colspan="5"] > em
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: color-contrast - Elements must have sufficient color contrast
08:56:50 	In element: .region-content_below-message > td[colspan="5"] > em
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: color-contrast - Elements must have sufficient color contrast
08:56:50 	In element: .region-footer_top-message > td[colspan="5"] > em
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-header > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-primary_menu > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-secondary_menu > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-hero > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-highlighted > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-breadcrumb > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-social > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-content_above > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-content > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-sidebar > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-content_below > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-footer_top > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: duplicate-id-active - IDs of active elements must be unique
08:56:50 	In element: .region-title-footer_bottom > td[colspan="5"] > .region-title__action.js-form-wrapper.form-wrapper
08:56:50   ✖ NightwatchAssertError
08:56:50    aXe rule: region - All page content should be contained by landmarks
08:56:50 	In element: #secondary-tabs-title

Things to consider:

It may be easier to split this into multiple issues
These tests were performed with Claro as theme, so it is possible some A11y fails are Claro specific, vs. originating from the Block module. For any given issue where that is the case, change the Component to Claro.
If any of the a11y errors are considered acceptable (either false positives or there's a tradeoff that provides greater benefit), it's possible to leave something unfixed, but the rationale should be documented within the code.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Add a classloader that can handle class moves

catch — Tue, 28 Jan 2025 16:50:21 +0000

Problem/Motivation

Sometimes we want to move a class from one place to a different place. An example is in #3502602: Remove non-formatter plugins out of the file field formatter directory.

There are two solutions we've used in the past:

1. Leave the old class in place, usually extending the new class, add @deprecated and trigger a deprecation. When we want to move something to avoid it being scanned for discovery, this does not help us until the next major release.

2. Add a call to class_alias - this has to be manually specified in boostrap.inc and can never be removed.

I think we might be able to handle both deprecation and aliasing in a classloader.

Steps to reproduce

Proposed resolution

Something like:

1. Create a new classloader

2. This classloader allows a $movedClasses array to be populated via a setter.

3. The moved classes array is keyed by the moved class, the value is an array of the new classname, the deprecated from version, the removed version (latter two could be optional).

4. in DrupalKernel, register this as an extra classloader, it should run after the composer classloader so it has zero runtime overhead.

5. When the composer classloader can't find a class, PHP falls back to our classloader. Our classloader checks the $movedClasses property, if the class is in there, it calls class_alias($old_class, $new_class, TRUE);

If this works, we could then look at populating the moved classes array, potentially from a container parameter so that it's possible for core and contrib modules to add to it.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Get nightwatch a11y test coverage to use Standard profile

the_g_bomb — Wed, 22 Jan 2025 17:33:31 +0000

Problem/Motivation

Given that Nightwatch has been udpated to include axecore in the work done in #2857808: Automate Accessibility Checks for Core
Resulting in: #2857808: Automate Accessibility Checks for Core

It would be good to continue to cover the rest of the pages highlighted in phase 1:
#2857808: Automate Accessibility Checks for Core

This ticket is to change the test to use the standard installation profile to allow more pages to the covered.

This will introduce the installation profile, but add bypasses to any tests that fail to allow the test to be added.

As the issues are fixed we can remove the skip rules.

$query->addExpression and $query->orderBy in views Sql

Andrew211 — Wed, 01 Nov 2017 05:46:57 +0000

Currently the views SQL class 'core/modules/views/src/Plugin/views/query/Sql.php Sql' has limitations in the sense that all joins, order by, group by, expressions, etc must belong to a group or field.

It would be nice to be able to have the same freedom when building/modifying queries as the Select class offers. (core/lib/Drupal/Core/Database/Query/Select.php Select)

eg. there is currently no way to call "addExpression" via hook_views_query_alter.

Yes we can call "addHavingExpression" or "addWhereExpression", however no way to add a select expression that is not related to a field.

Something like this would do the trick

public function addCustomExpression($string, $alias){
    $this->customExpressions[] = [
      'string' => $string,
      'alias' => $alias
    ];
  }

Then when building the query, something like this

protected function compileFields($query) {
    if(count($this->customExpressions) > 0){
      foreach($this->customExpressions as $ex){
        $query->addExpression($ex['string'], $ex['alias']);
      }
    }

Then of course a programmer may wish to order by this field, so we'd need some sort of custom order by to order by the field. Perhaps something like the below

public function addCustomOrderBy($field, $direction){
    $this->customOrderBys[] = [
      'field' => $field,
      'direction' => $direction
    ];
  }

Then inside the 'compileFields' method.

if(count($this->customOrderBys) > 0){
      foreach($this->customOrderBys as $ob){
        $query->orderBy($ob->field, $ob->direction);
      }
    }
}

I understand views is field based, however it would be nice to give developers the freedom to manipulate the sql to their choosing.

Cheers.

Migrate Feed Icon SDC component

ignaciofarre — Sun, 26 Jan 2025 12:54:16 +0000

Problem/Motivation

Drupal core currently lacks a reusable "Feed Icon" component,. This results in inconsistent styling and redundant implementations across the site. The creation of a standardized "Feed Icon" SDC will ensure design consistency, reusable code, and improved maintainability. This component will also enhance accessibility and will align with the design system for a cohesive user experience.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Convey AJAX progress messages to assistive technology.

andrewmacpherson — Tue, 15 May 2018 20:39:51 +0000

Problem/Motivation

The AJAX API has a feature to display a progress message, but it isn't conveyed to assistive tech like screen readers.

'#ajax' => array(
  'callback' => 'Drupal\config_translation\FormElement\DateFormat::ajaxSample',
  'event' => 'keyup',
  'progress' => array(
    'type' => 'throbber',
    'message' => t('Loading more products'),
  ),
),

Proposed resolution

Find a way to convey the progress message.
Possibilities (use one approach only, not both):

Mark the visible AJAX message as an ARIA-live region (preferred?), or
Duplicate the visible message using a Drupal.announce() call.

Other possibilities might be to provide incrementally updated messages, like "Updating... 30%... 80%... finished".

Remaining tasks

Perform manual testing
Perform accessibility review

User interface changes

No visual changes. Format the AJAX message so it is conveyed to assistive technology, e.g. so screen readers can announce it.

API changes

Several screenreader related properties have been added to the AJAX Progress settings.

announce - Determines if screenreaders should announce progress for this operation
announceDelay - The delay in milliseconds before the screenreader announces the progress
announceIntervalTime - The interval in milliseconds between each announcement
announceMessage - The message to be announced by the screenreader, if no message property is set.

Data model changes

No data model changes introduced.

Background reading

How do you make the Loading icon accessible for screen-readers like JAWS?
Accessibility: Page Loader indicator using aria-live
JavaScript and Accessibility: Don't Blame the Language (DrupalCon Nashville 2018 session). Recording and slides from Everett Zufelt's talk.
JavaScript and Accessibility: Don’t Blame the Language, Everett Zufelt's article version on Medium.
aria-live: the good, the bad and the ugly . Includes test notes about aria-live support as of 2015.
Injecting content into the page tests. The same test results from the previous slide deck.
Live Region Playground. A cool tool from Deque, useful for testing support on current browser/OS/AT combinations.
Testing ARIA-LIVE with display:none and innerHTML
ARIA Live Region Test. Terrill Thompson's test page. Not sure what date, but it does have versions.

Commit credits

@bgrobertson, who reported this issue on the #accessibility channel on Drupal Slack, and researched which bits of the AJAX API were relevant. This issue report is a summary of his comments there.
@ckueda, who provided a patch on the duplicate issue #2907132: Indicate ajax throbber activity to assistive technology, which is broadly similar to the earliest patch here.

Deprecate comment_uri()

internetdevels — Sun, 02 Jun 2013 14:28:34 +0000

Problem/Motivation

Part of #2010184: [meta] convert ‘uri_callback’ entities param to EntityInterface::uri() method

Current state: the functions is not used after #1970360-78: Entities should define URI templates and standard links

Needs to implement Plugin\Entity\Comment.php uri() method and deprercate comment_uri() function replacing it's calls to $comment->uri() and clean-up Comment annotation uri_callback

1 usage in contrib found http://grep.xnddx.ru/search?text=comment_uri&filename=

Proposed resolution

- deprecate comment_uri() in favor of Comment::toUrl()
- probably it needs to update docs because Comment::permalink() should be deprecated too #2113323: Rename Comment::permalink() to not be ambiguous with ::uri()

Remaining tasks

Investigate removal of uri_callback, git grep uri_callback returns usages.

User interface changes

API changes

deprecated comment_uri()

Data model changes

Release notes snippet

Document the return value expected by SelectionInterface::getReferenceableEntities() when used on an entity type with no bundles.

jludwig — Sat, 25 Mar 2017 00:02:28 +0000

Issue description

The documentation in Drupal\Core\Entity\EntityReferenceSelection\SelectionInterface::getReferenceableEntities() doesn't specify the expected first level key when used on an entity type that does not contain bundles:

  /**
   * Gets the list of referenceable entities.
   *
   * @return array
   *   A nested array of entities, the first level is keyed by the
   *   entity bundle, which contains an array of entity labels (escaped),
   *   keyed by the entity ID.
   */
  public function getReferenceableEntities($match = NULL, $match_operator = 'CONTAINS', $limit = 0);

The expected first level key when no bundles are available is the entity type ID; example: 'role'.

Without that first level entity type id key, any many other functions using this method will throw PHP notices etc; example: core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem::getSettableOptions().

If this had been documented, I would have known that my implementation of getReferenceableEntities() for a custom entity type was incorrect rather than assuming getSettableOptions() was incorrect and writing a patch etc. Adding a note will likely save others time debugging the PHP notices from not returning the expected value in getReferenceableEntities().

Proposed text for @return documentation

A nested array of entities, the first level is keyed by the entity bundle (or the entity type ID if no bundles exist), which contains an array of entity labels (escaped), keyed by the entity ID.

Add Date range filters for Moderate Content

anjaliprasannan — Mon, 27 Jan 2025 13:26:17 +0000

Problem/Motivation

The existing filtering options in the "Moderate" tabs are limited, making it difficult for users to find specific content efficiently.

Steps to reproduce

The filtering options in the "Moderate" tabs could really use an upgrade! Adding filters like a date range picker would be a game-changer, making it way easier to sort through and find content by specific time periods.

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Introduce the concept of "specificity" for MENU_LOCAL_TASK and disallow "duplicates"

donquixote — Tue, 26 Jan 2010 00:21:40 +0000

In a custom module I have a hook_menu implementation with the following entries:
(adapted for better understanding. "xxx" was actually "mlid/%menu_link")

xxx : MENU_CALLBACK
xxx/under-construction : MENU_DEFAULT_LOCAL_TASK
xxx/add/page : MENU_LOCAL_TASK
xxx/add/%node_type : MENU_LOCAL_TASK

The intention:

On xxx/under-construction, I want to see two tabs: The active one ("under construction") and the one for xxx/add/page.
On xxx/add/page, I want to see two tabs: The active one ("add page") and "under construction".
On xxx/add/webform, I want to see three tabs: xxx/under-construction, xxx/add/page, and xxx/add/webform.

For 1) and 3) this works as intended, just for 2) I see a duplicate tab for "xxx/add/page". Obviously, the first is created for the router item "xxx/add/page", and the second one is for "xxx/add/%node_type".

So, my thinking:
- Yes, I am using the system in a legitimate way. Router items with different argument masks are allowed to coexist.
- There is no point in having two tabs with the same link path. The more explicit router path (with fewer "%") should have priority, the other should be ignored.

Technical obervation:
The most important things happen in menu_local_tasks(), around this place:

<?php
    while ($item = db_fetch_array($result)) {
      _menu_translate($item, $map, TRUE);
      if ($item['tab_parent']) {
?>

(my line numbers are probably incorrect due to some patching)

I have not tried this with D7, so I set the version to 6.x-dev. Feel free to change this, if you can confirm it for D7 !

Cache backend consistency runtime check and graceful downgrade mecanism

pounard — Sat, 21 May 2011 12:01:00 +0000

Hello,

Cache backend management is a low-level critical functionallity, doing the Cache Backport module, I also added some code improvements into the D6 backport.

I think Drupal 7 could benefit those improvements, since they are API compatible and could avoid some crashes on misconfigured environments.

Basically, I do all of this:

Adding an interface for strongly environment related backends, backends that needs external libraries to run steamlessly should implement a new checkEnv() method in order for core to test if the environment if ready.
I added a dummy implementation of this method on the database backend, as a code sample.
I added a class_exists() call at cache backend instanciation time, if the current class does not exists, the backend gracefully downgrade to default.
Added the necessary checkEnv() calls while instanciating the backend class, and provide a graceful downgrade to default if it fails.
Added a bonus Null Object implementation of cache backend; this allows site administrators or developers to disable fully cache for one or more bins by configuration.

Using this interface improvements, it can be easy to implement administration screens (at least report screen) for various cache backends state, what I did in the D6 backport, and I could easily do for D7 if people are interested.

Cache backend being plugins is now being discussed for D8, so that's why I won't do a D8 patch until the solution to this problem comes up, until, this can still provide a nice consistency bugfix for D7 next release.