🇺🇸United States @benjifisher

We discussed this issue at 📌 Drupal Usability Meeting 2025-10-24 Active (@rkoller and @simohell) and 📌 Drupal Usability Meeting 2025-10-31 Active (@benjifisher, $rkoller, and @simohell). Those issues each have a link to a recording of the meeting. I am giving issue credit here to the attendees at the usability meetings.

This comment is not a usability review. Instead, it is about the results of some manual testing. (At the second meeting, we were a little confused: when we did not see data loss, we thought the issue was outdated. We forgot that we should have tested with 11.x instead of the branch from the MR.)

The issue summary refers to 🐛 Deleting a Media view mode should not result in the deletion of an entire text format Active . That issue was closed as a duplicate of 🐛 Config entities implementing EntityWithPluginCollectionInterface should ask the plugins to react when their dependencies are removed Needs work , which is now Fixed. We have not tested to confirm, but it seems as if the only ways to delete a filter format are using PHP code (as in the Steps to reproduce) or perhaps using the jsonapi module. I am adding the tag for an issue summary update and setting the status back to NW. Perhaps the priority should be downgraded from Critical, too.

The Steps to reproduce are incomplete if there is more than one field using the same field storage. For example, this is the case when testing with the Standard or Umami installation profile. If only one of the field configurations is removed, then the field storage configuration is not removed, and that means that the database table is not removed. In this case, it is possible to restore the data from the deleted field:

1. Import configuration to restore the field, along with the entity form and display modes. (Untested, but it is probably has the same effect if you create a "new" field with the same machine name through the UI.)
2. Make sure the field is configured to allow text formats that have not been deleted.
3. Update the database table, setting deleted = 0 for all rows (or only the ones affected by deleting the filter format, if you can think of a way to do that).

Related: the issue summary states,

Reason is that the field storage has a config dependency on the filter format, when it is configured as allowed-format.

The field, not the field storage, has a dependency on the filter format.

I also made some comments on the MR. These are not related to the usability meeting, and they are just my own opinions.

Seeing no objections to Comment #37, I am closing this issue. I transferred credit to the other issue. From my comment there:

I may be too stingy: I did not give credit to the authors of Comments 2, 4, 5, 6, 10, 12 , 13, 16, 18, nor myself.

I am giving credit on this issue to people who contributed to 🐛 Link field description squashes the field Active . I may be too stingy: I did not give credit to the authors of Comments 2, 4, 5, 6, 10, 12 , 13, 16, 18, nor myself.

For testing purposes, see Comment #34 (@marcoliver) from that issue:

I have now also attached the quick and dirty module → I whipped up for testing. It basically just does some preprocessing and attaches itself to any field whose machine name starts with field_related_.

(I took the liberty of making "quick and dirty module" a link to the zip file.)

@kentr:

Thanks for pointing me to claro_system_module_invoked_library_info_alter() in Comment #60. That function was added in #3070493: Introduce a mechanism to provide an alternate Claro design for the toolbar in the future → (odd title). The idea is that Claro (the back-end theme) should be able to style the toolbar even on front-end pages. Personally, I think a better idea would have been to provide a mechanism for the site admin to choose whether the front-end theme or the back-end theme (a.k.a. the default theme or the admin theme) gets to style the toolbar on front-end pages.

I do see the value in making the toolbar consistent on all pages.

Anyway, I just added a few lines to that function so that Claro insists on using toolbar.menu.js from the toolbar module (just as it insists on using various CSS files from Claro). It takes just a few lines, but I could not get it right the first time I tried, about a week ago.

I am adding a few lines to the issue description and setting the status back to NR.

It looks as though git.drupal.org is in the middle of a 2-hour maintenance window, so I cannot check the pipeline.

The failing test is because of 🐛 Initial workspace-published revisions are marked as new too late Needs review . It also fails on the 11.2.x branch.

There were only 5 commits on MR !12792. (See Comment #35.) I cherry-picked the first three to the 11.2.x branch and added https://git.drupalcode.org/project/drupal/-/merge_requests/13561.

I have not done any testing.

The child issue #3550720: Refactor FieldUiJSTestTrait::fieldUIAddNewFieldJS() because it is brittle → was just Fixed, and I have updated the MR here. Judging by line count, it is a small improvement: +381/-152 instead of +395/-175. But the part that is handled by the child issue is very different from the rest of the MR, so I think the actual simplification is more than the line count suggests.

I am also updating the issue summary, since the Proposed resolution section no longer has to mention the change that was handled by the child issue. While I am at it, I am updating the last paragraph of the Proposed resolution section based on recent updates to the MR. (To do: update the screenshot.)

In terms of managing the issues, I think it makes sense to close this one as outdated and add credit for everyone who contributed here to #3471459.

The hard part will be updating the MR here, which already has a lot of work put into it. (And then, not so hard, attaching that MR to the other issue.)

benjifisher → created an issue.

@smustgrave:

Probably not a coincidence: when reviewing the parent issue, @dww had several suggestions for the changes to FieldStorageAddForm. Most of my changes to that file are refactoring a long function, so @dww was actually suggesting improvements to the existing code, which I think is out of scope (for the parent issue). I think most or all of that code came from the same #3386762 that you mentioned.

I should have left the status at NW when I commented on the MR.

@marcoliver:

... and made the proposed wrapper element conditional so it only appears if a prefix or suffix is set.

+1 for that. And thanks for the sample code in Comment #34.

After 📌 Drupal Usability Meeting 2025-10-10 Active and 📌 Drupal Usability Meeting 2025-10-17 Active , @rkoller and I updated the MR. We also replied here on the issue and on every open thread on the MR. (In Comment #91, I wrote, "I do not plan to reply to each point on the MR", but I changed my mind.

@dww, thanks again for your initial review. I hope you can find time for a second round.

On https://git.drupalcode.org/issue/drupal-3370326/-/jobs/6949568,

• Drupal\Tests\layout_builder\FunctionalJavascript\BlockFilter failed.
• Drupal\FunctionalJavascriptTests\Theme\OliveroAvoidStorageUsingTest errored.

On https://git.drupalcode.org/issue/drupal-3370326/-/jobs/6949569 (part of the same pipeline)

• Drupal\Tests\views\FunctionalJavascript\Plugin\views\Handler\GroupedExposedFilterTest failed

We do not open a new issue after one random failure, right? But if anyone else sees these tests failing, then we should.

I re-ran both jobs, and now ... my pipeline is green. :)

Since we want to fix this issue without breaking 🐛 Prefix/Suffix not inline with autocomplete field Active , we need to test that issue. Since adding prefix and suffix requires some custom code (a form alter or a preprocess function) it would be really helpful if someone could share some sample code for that.

Also, that issue refers to autocomplete fields with prefix and suffix. Maybe all we have to do here is make the selector more specific: td > .form-item.form-type--entity-autocomplete. (And make the same change to the two related rules added in #3471459.) Is that sort of like using the BEM strategy? (I ask because I am a back-end developer.)

benjifisher → changed the visibility of the branch 3519949-link-field-description to hidden.

I ran into this issue while testing 📌 [PP1] Refine field descriptions Active . I added a Datetime field (stored as a timestamp) with unlimited values. See what happens when I add an additional value to this multi-valued field:

The title of this issue mentions Link fields, but the bug also affects Datetime fields, maybe all fields that have description text, and the Permissions form (Comment #21). I am updating the title of this issue to reflect that. I will update the issue description later, unless someone else does that first.

I am also hiding MR !12124. See Comment #7: it breaks what 🐛 Prefix/Suffix not inline with autocomplete field Active was trying to fix.

benjifisher → created an issue.

I think you are right: we have to fix this. I am setting the status back to NW.

I do not mind a little redundancy, but my first thought is that visually-hidden should be enough. I will look into why it is not, but it might be a few days before I get to it.

@kentr:

I confess: I often have to remind myself that the T in RTBC stands for "tested". Thanks for the detailed test results on this issue.

When I follow your steps to reproduce (STR), I see the same result on admin and non-admin pages: I see the text, and my browser tools do not show any setting for text-indent.

In Step 4, if I also set stable9 as the admin theme, then I get the expected result. I no longer see the text (admin page or not) and my browser tools show text-indent: -9999px on the <button> element. Based on that, I am setting the status back to NR.

It seems you are right, and some of the Claro styles are leaking (or bleeding) into the toolbar. That could be a known bug, like #2987665: Toolbar styling is easily disrupted by theme CSS. → . I thought there was an issue like 🐛 Front-end theme styles can bleed into Navigation Active for the toolbar, not Navigation, but maybe I was wrong. Since it seems like the admin theme is bleeding into the toolbar, this might be a new bug instead of an old one.

Would it make sense to put the current list of tests with intermittent failure into a special test group for GitLab CI? It would require some maintenance, but I think it would save time in two ways:

1. When a test fails, I do not have to come to this issue and see whether it has been reported. I will know from the GitLab UI that re-running the test is the right thing to do.
2. When I re-run the test, I will not have to re-run all the well-behaved tests in the existing test group.

I plan to continue the discussion from the 2025-10-10 meeting. See the long comment there.

I suggested this on Slack, and @rkoller agreed. Let's get some other opinions before making this change.

Looking again at the telephone field, I think we should strike "Store" from

> Store a telephone number

But then we are left with just "a telephone number". Usually, if the description does not say any more than the label, it is best to leave the description off.

Perhaps we should say something like this instead:

> A telephone number, stored as a simple string.

That will clarify that Drupal does not store it as any sort of complex data structure. It does not store country code, area code, or anything else. That is kind of unfortunate, since it limits what you can control when displaying it.

I do not see anything in the screenshots (the original nor the updated one) to indicate which fields are required. Am I missing something?

I generally prefer to put admin paths in <code> tags. In this case, it is especially helpful since the unknown (and unclosed) <content_type> tag was being stripped.

We discussed this issue at 📌 Drupal Usability Meeting 2025-10-10 Active . That issue has a link to a recording of the meeting. I am giving issue credit here to the attendees at the usability meeting: benjifisher, jannafiester, joannajackson, rkoller, and simohell.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

In preparation for the meeting, I went through all the text changes suggested or requested by dww, in the order they appear on the MR. I figured out where they appear in the UI and prepared the list below (copied from #3550127). During the meeting, we got through about half of the list: we got as far as Media. We will update the MR based on the discussion, but I do not plan to reply to each point on the MR. @dww, If you still object or have questions, then please either bump the thread (i.e., add another comment) or look at the recording. Or maybe it is simpler if you just resolve the threads where you are satisfied with our choice, and then I will reply to the unresolved threads.

@dww reviewed the MR for 📌 [PP1] Refine field descriptions Active . The review included some technical points and some suggestions for improving the interface text. The following sections summarize those suggestions. Each numbered list includes

1. The text in 11.x.
2. The text in the current MR.
3. The suggestion or comment from @dww.

File Upload > File

1. • For uploading files
   • Can be configured with options such as allowed file extensions and maximum upload size
2. • Downloadable files
   • Configure allowed file extensions, maximum upload size, and storage location
   • Examples: document (pdf, txt), audio (mp3), video (avi, mov), image (jpeg, png, webp, gif), etc.
3. Given we have a separate Image field type, why does the help text for this specifically recommend using a generic File to upload image file types?

Date and Time

1. Field to store date and time values.

• A date and/or time value
• The field types use different formats to store date and time in the database. All field types can be displayed in various ways using the Manage display tab.’

1. The 2nd sentence here is true of all fields, not just date_time, right? Do we want this here?

Formatted Text > Long text with summary

1. • Ideal for longer texts, like body or description with a summary
   • Allows specifying a summary for the text
   • Supports long text without specifying a maximum length
   • May use more storage and be slower for searching and sorting
2. • Extends Long text
   • Uses separate text areas (multiple rows) for input and summary
   • Summary defaults to a trimmed value of the long text
   • Recommended for content types that will be displayed in cards or teasers
3. I thought we’re trying to deprecate this field type. 😅 Not recommended for anything? 😆

I do not think this is a serious suggestion.

Telephone Number

To see this option, enable the telephone module.

1. This field stores a telephone number.
2. Store a telephone number with an output link option
3. Store a telephone number with an option to output as a link

Same concern as with email about what an "output link option" is.

Selection List > Text

1. • Values stored are text values
   • For example, ‘US States’: IL => Illinois, IA => Iowa, IN => Indiana
2. • Values (machine names) are text strings
   • By default, the Value is based on the Name
   • Example Name (Value): Apple (apple), Mango (mango), Black Cherry (black_cherry)
   • Example Name (Value): Most (0.9), Half (0.5), Some (0.3)
3. • Example: Apple (apple), Mango (mango), Black Cherry (black_cherry)

I know "0.9" is technically a "text string", but this seems confusing.

Selection List > Integer

The Name (Value) part of this is a bit confusing. Maybe just:

1. • Values stored are numbers without decimals
   • For example, ‘Lifetime in days’: 1 => 1 day, 7 => 1 week, 31 => 1 month
2. • Values are whole numbers
   • Example Name (Value): Disagree (-1), Neutral (0), Agree (1)
3. • Example: Disagree (-1), Neutral (0), Agree (1)

Reference > Content

1. An entity field containing an entity reference.
2. • Link content, such as Basic page or Article
   • Examples: related articles, next/previous links
3. • Link content

These node types may or may not exist on any given site. Is this a good idea?

Media

1. • Field to reference media. Allows uploading and selecting from uploaded media.
2. • Create media type with the ability to upload or link assets Maybe just this:
3. • Reference a media item, either a new upload or link existing

Date and Time

1. • Ideal when date and time needs to be input by users, like event dates and times
   • Date or date and time stored in a readable string format
   • Easy to read and understand for humans
2. • Human-readable string format
   • Input requires: date
   • Example: 2001-01-14T00:00:00
3. This is called a "Datetime", but "Input requires: date"? That’s confusing. Plus the example shows a time. What value is "Input requires: date" adding here? Less is more? Remove it?

Plain Text > Long Text

1. • Ideal for longer texts, like body or description
   • Supports long text without specifying a maximum length
   • May use more storage and be slower for searching and sorting
2. • Uses a text area (multiple rows) for input
   • No fixed maximum length
   • May use more storage and be slower searching and sorting
   • Recommended for longer texts, like body
3. I thought we’re generally moving away from a "body" field in core. Do we even need this line at all? This option is labeled "Long text". Doesn’t that imply it’s recommended for long text?

Number > Integer

1. • Number without decimals
   • For example, 123
2. • Whole numbers
   • Values are positive, negative, or zero
   • Maximum values depend on the system
   • Examples: 123, -123, 0, 3 (pi)
3. • Examples: 123, -123, 0

3 != pi. 🤓 😆 This is a weird example.

Isn’t the 2nd line ("Values are positive, negative, or zero") a definition of the 1st ("Whole numbers")? Do we need to say both?

Sorry, just double checked myself. "Whole number" is defined as a positive integer or 0.

Negative integers are not whole numbers. This new help text is definitely wrong. Perhaps the original text "Numbers without decimals" is the shortest, most clear.

@rkoller:

looking at https://www.britannica.com/dictionary/eb/qa/what-is-an-integer how about a small addition to yours and replace "Whole number" with "Number without decimal parts or fractions" instead?

Number > Float

In order to see this option, you have to remove the line

  no_ui: TRUE,

in core/lib/Drupal/Core/Field/Plugin/Field/FieldType/FloatItem.php.

1. • In most instances, it is best to use Number (decimal) instead, as decimal numbers stored as floats may contain errors in precision
   • This type of field offers faster processing and more compact storage, but the differences are typically negligible on modern sites
   • For example, 123.4 km when used in imprecise contexts such as a walking trail distance
2. • Numbers with decimal parts and exponents
   • Only limits provided by the system
   • Examples: 1.23, -1.23, 0.00e1, 1e100, 6.02214076e23
3. I don’t know what "Only limits provided by the system" means, using either new nor "old" eyes. 😆

Reference > ???

You only see this default value if a module declares a reference type to be "common" but does not provide a description. For testing purposes, you can remove the description from core/modules/node/src/Hook/NodeFieldHooks.php.

1. • An entity field containing an entity reference.
2. • A reference to a(n) @item
3. I know we don’t know what @item will be, but a(n) is a bit awkward. Again, not sure what to suggest.

Reference > Other

1. • An entity field containing an entity reference.
2. • Choose from all available options on the next screen
   • Any content or configuration entity could be referenced
3. Again, with "new eyes", WTF is a "configuration entity"? 😅 Not sure what to suggest here.

@rkoller:

we’ve simply used the terminology that is used in the next step on the field settings. if you take a look at the "type of item to reference" select then you see that the options are grouped by "content" and config". that was the reasoning why we went with "…content or configuration entity…"

Email

1. • Field to store an email address.
2. • An email address with an output link option
3. • An email address with an option to output as a link

I know we’re attempting to be terse, but trying to put on my "new eyes", I’m not sure I’d understand what "an output link option" is.

benjifisher → created an issue.

@dww reviewed the MR for 📌 [PP1] Refine field descriptions Active . The review included some technical points and some suggestions for improving the interface text. The following sections summarize those suggestions. Each numbered list includes

1. The text in 11.x.
2. The text in the current MR.
3. The suggestion or comment from @dww.

File Upload > File

1. • For uploading files
   • Can be configured with options such as allowed file extensions and maximum upload size
2. • Downloadable files
   • Configure allowed file extensions, maximum upload size, and storage location
   • Examples: document (pdf, txt), audio (mp3), video (avi, mov), image (jpeg, png, webp, gif), etc.
3. Given we have a separate Image field type, why does the help text for this specifically recommend using a generic File to upload image file types?

Date and Time

1. Field to store date and time values.

• A date and/or time value
• The field types use different formats to store date and time in the database. All field types can be displayed in various ways using the Manage display tab.’

1. The 2nd sentence here is true of all fields, not just date_time, right? Do we want this here?

Formatted Text > Long text with summary

1. • Ideal for longer texts, like body or description with a summary
   • Allows specifying a summary for the text
   • Supports long text without specifying a maximum length
   • May use more storage and be slower for searching and sorting
2. • Extends Long text
   • Uses separate text areas (multiple rows) for input and summary
   • Summary defaults to a trimmed value of the long text
   • Recommended for content types that will be displayed in cards or teasers
3. I thought we’re trying to deprecate this field type. 😅 Not recommended for anything? 😆

I do not think this is a serious suggestion.

Telephone Number

To see this option, enable the telephone module.

1. This field stores a telephone number.
2. Store a telephone number with an output link option
3. Store a telephone number with an option to output as a link

Same concern as with email about what an "output link option" is.

Selection List > Text

1. • Values stored are text values
   • For example, ‘US States’: IL => Illinois, IA => Iowa, IN => Indiana
2. • Values (machine names) are text strings
   • By default, the Value is based on the Name
   • Example Name (Value): Apple (apple), Mango (mango), Black Cherry (black_cherry)
   • Example Name (Value): Most (0.9), Half (0.5), Some (0.3)
3. • Example: Apple (apple), Mango (mango), Black Cherry (black_cherry)

I know "0.9" is technically a "text string", but this seems confusing.

Selection List > Integer

The Name (Value) part of this is a bit confusing. Maybe just:

1. • Values stored are numbers without decimals
   • For example, ‘Lifetime in days’: 1 => 1 day, 7 => 1 week, 31 => 1 month
2. • Values are whole numbers
   • Example Name (Value): Disagree (-1), Neutral (0), Agree (1)
3. • Example: Disagree (-1), Neutral (0), Agree (1)

Reference > Content

1. An entity field containing an entity reference.
2. • Link content, such as Basic page or Article
   • Examples: related articles, next/previous links
3. • Link content

These node types may or may not exist on any given site. Is this a good idea?

Media

1. • Field to reference media. Allows uploading and selecting from uploaded media.
2. • Create media type with the ability to upload or link assets Maybe just this:
3. • Reference a media item, either a new upload or link existing

Date and Time

1. • Ideal when date and time needs to be input by users, like event dates and times
   • Date or date and time stored in a readable string format
   • Easy to read and understand for humans
2. • Human-readable string format
   • Input requires: date
   • Example: 2001-01-14T00:00:00
3. This is called a "Datetime", but "Input requires: date"? That’s confusing. Plus the example shows a time. What value is "Input requires: date" adding here? Less is more? Remove it?

Plain Text > Long Text

1. • Ideal for longer texts, like body or description
   • Supports long text without specifying a maximum length
   • May use more storage and be slower for searching and sorting
2. • Uses a text area (multiple rows) for input
   • No fixed maximum length
   • May use more storage and be slower searching and sorting
   • Recommended for longer texts, like body
3. I thought we’re generally moving away from a "body" field in core. Do we even need this line at all? This option is labeled "Long text". Doesn’t that imply it’s recommended for long text?

Number > Integer

1. • Number without decimals
   • For example, 123
2. • Whole numbers
   • Values are positive, negative, or zero
   • Maximum values depend on the system
   • Examples: 123, -123, 0, 3 (pi)
3. • Examples: 123, -123, 0

3 != pi. 🤓 😆 This is a weird example.

Isn’t the 2nd line ("Values are positive, negative, or zero") a definition of the 1st ("Whole numbers")? Do we need to say both?

Sorry, just double checked myself. "Whole number" is defined as a positive integer or 0.

Negative integers are not whole numbers. This new help text is definitely wrong. Perhaps the original text "Numbers without decimals" is the shortest, most clear.

@rkoller:

looking at https://www.britannica.com/dictionary/eb/qa/what-is-an-integer how about a small addition to yours and replace "Whole number" with "Number without decimal parts or fractions" instead?

Number > Float

In order to see this option, you have to remove the line

  no_ui: TRUE,

in core/lib/Drupal/Core/Field/Plugin/Field/FieldType/FloatItem.php.

1. • In most instances, it is best to use Number (decimal) instead, as decimal numbers stored as floats may contain errors in precision
   • This type of field offers faster processing and more compact storage, but the differences are typically negligible on modern sites
   • For example, 123.4 km when used in imprecise contexts such as a walking trail distance
2. • Numbers with decimal parts and exponents
   • Only limits provided by the system
   • Examples: 1.23, -1.23, 0.00e1, 1e100, 6.02214076e23
3. I don’t know what "Only limits provided by the system" means, using either new nor "old" eyes. 😆

Reference > ???

You only see this default value if a module declares a reference type to be "common" but does not provide a description. For testing purposes, you can remove the description from core/modules/node/src/Hook/NodeFieldHooks.php.

1. • An entity field containing an entity reference.
2. • A reference to a(n) @item
3. I know we don’t know what @item will be, but a(n) is a bit awkward. Again, not sure what to suggest.

Reference > Other

1. • An entity field containing an entity reference.
2. • Choose from all available options on the next screen
   • Any content or configuration entity could be referenced
3. Again, with "new eyes", WTF is a "configuration entity"? 😅 Not sure what to suggest here.

@rkoller:

we’ve simply used the terminology that is used in the next step on the field settings. if you take a look at the "type of item to reference" select then you see that the options are grouped by "content" and config". that was the reasoning why we went with "…content or configuration entity…"

Email

1. • Field to store an email address.
2. • An email address with an output link option
3. • An email address with an option to output as a link

I know we’re attempting to be terse, but trying to put on my "new eyes", I’m not sure I’d understand what "an output link option" is.

@ultimike:

Thanks for the review!

Looking at the MR, it looks like it isn't exactly a revert ...

I had forgotten that I kept the test that @abhijith s (or @AbhijithS ... one is d.o, one is GitLab) wrote. As I said in #20, it is a very nice test.

The third commit is a clean revert (git revert ...).

See my Comment #26 for why the later commit was needed.

I think we should focus on getting this issue to RTBC.

The core committers talk to each other. If any one of them decides to review this issue, and they want to ask @nod_ about it, then they will.

I added #3550720: Refactor FieldUiJSTestTrait::fieldUIAddNewFieldJS() because it is brittle → with just the changes to the test trait. I think that reduces the scope of this issue, making it easier to review. It also gives us a MR to prove that this change does not break anything.

If the child issue gets fixed, then we can remove (4) from the Proposed resolution of this issue.

benjifisher → created an issue.

The pipeline passed.

I mentioned the discussion of .getAccessibleName() in Comments #47, #49 to #54 in the Proposed resolution section.

@kentr: FYI, this attention to the issue summary is one way I try to make it easier for the core committer to do the final review.

There is a test failure for core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php. It looks unrelated, and the test passes locally, so I am re-running the test.

I got a test failure for core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5Test.php on https://git.drupalcode.org/issue/drupal-3093378/-/pipelines/619981.

I am adding a little more detail to the Proposed resolution section of the issue summary.

@kentr:

Thanks for asking about my health. At this point, I am working three days a week and getting back to Drupal contribution. In a few months, I should be close to my "old normal".

If you are willing to take on the Reviewer role, then I will defer to you. I reverted your revert. Of course, the core committer will have the last word.

Referring to translated text:

Wouldn't the inner text content that is found and checked by .textEquals() also be affected by this?

Good point.

@cwilcox, thanks for weighing in.

Now the pipeline is failing ...

I will see if I can fix it. Maybe Tuesday.

I hope you will reach out to @larowlan and/or @quietone on Slack and discuss the possibility of large-scale changes to the tests.

Returns the computed WAI-ARIA label of an element.

If we translate the text, either using $this->t() in PHP code or Drupal.t() in JS, won't that be the translated text? Again, I was unable to try this out since I could not get the tests to run locally. I suppose I could push a temporary, do-not-merge commit and let GitLab CI do the test.

Thanks for reverting your commit.

I see myself more as a collaborator than a sole reviewer.

That's a bummer. Then we still need to find someone to review the issue.

Or maybe this is the time for you to take on the Reviewer role, which is very important. I think you are familiar with the code and test changes on this issue. Why not review it?

RTBC means Reviewed and Tested By the Community, not Ready To Be Committed. (It used to stand for that.) You are a member of the community. Your review represents your best effort to improve, then approve the issue. Do your best to ask the questions that a core committer would ask. Your best effort is good enough. If there is part of the MR that you are unsure of, be open. Tell them that you are not sure, and they will focus their attention in the right place. Some of my early reviews admitted that I had not looked closely at the tests, since I did not feel qualified. That's OK. Open Source Software is built on radical honesty and radical modesty.

When the core committer reviews the issue and asks for changes, you will learn something. Your next review will be closer to the goal of asking all the questions the committer would ask, which saves them the trouble of asking those questions.

@kentr:

I am sorry to be quiet for so long. I have been dealing with some serious medical issues. (If you want the details, then I can point you to a Slack thread.)

There are a couple of reasons I am nervous about using .getAccessibleName().

First of all, I am having trouble running the Nightwatch test locally. (See my Comment #46: "It took two tries, ...".) So I cannot tweak and evaluate as I would like.

Second, I searched the codebase and this seems like the first use of that test method. Maybe there is a reason it is not being used? Or maybe it should be used widely, in which case it might be a better idea to have a big issue for updating lots of tests instead of starting small with the test for this issue. In other words, I see a lot of value in keeping a single test self-consistent. Even if .getAccessibleName() is a better way to test, it might not be worth the inconsistency.

Third, I think .getAccessibleName() relies on the translated message. The usual practice in automated tests is not to use translated text unless you are testing translations. But see my first point above: I may be wrong about this.

There is also a meta-reason for not making this change. Until you made that test change, you preserved your role as a reviewer on this issue. I think we can restore that if we revert your commit. Then you have the authority to declare this issue RTBC. If we have both committed code to the MR, then we have to find someone else to review it.

I had an interesting discussion with @larowlan a while ago: https://drupal.slack.com/archives/C223PR743/p1753146459028029. I started by asking for some general advice on Cypress tests for the current project at my day job. Here is a bit of that thread:

A day or two ago, I was looking at some Nightwatch tests in the tooolbar module. They seem to use CSS selectors and nothing else.

-- @benjifisher

and

yeah so do a lot of our JavascriptFunctionalTests in core unfortunately

-- @larowlan

I was referring to the tests for this issue, of course.

Anyway, my point is that there is a lot of room for improvement in our current test suite. Maybe .getAccessibleName() is part of it, maybe not. But this is what I am thinking in my second point, where I suggest making broad changes to the tests instead of an isolated change to the test for this issue.

@kimpepper:

I see that you updated the MR. Should the issue status now be NR instead of NW?

@camhoward:

I am happy to discuss at length. It is unusual to revert an issue that has been committed and added to a full release, so we should be sure we know why we are doing it.

I have updated the issue summary, indicating the preferred approach. Are you willing to mark the issue RTBC? If you are not comfortable giving a code review, then maybe you can bring this issue up in DrupalEasy office hours (Comment #21) again and find someone who can do that.

Is there any advantage to reverting the change from #2219393 and then adding weights to the 32 YAML files?

My opinion is that reverting #2219393 is the correct fix for this issue. After that, if we want to add weights to some YAML files, then we can do that in separate issues: it seems out of scope for this one.

I had to re-read the comments to see that your "32 YAML files" refers to the second part of my Comment #22. I should have been clearer there: I was describing what we would have to do if we kept the alphabetical sort. My main point is that we should remove the alphabetical sort, and then we do not have to edit any YAML files.

@nicxvan, thanks for providing instructions on this novice issue!

I just want to add my thanks, too, since I am the one who created the issue.

You might get more help for questions like this on the #migration Slack channel. (If you do that, then please copy some of my reply to the thread. That will help avoid duplication of effort.)

I am afraid you are right, and adding another plugin is not going to work. The reason is that each process plugin has an input value (source) and configuration. The input value can come from an earlier step in the process pipeline. The configuration is static. Everything under attribute_map is configuration.

On the other hand, you keep using process: default_value. The other option (looking at the doc block of the plugin) is process: array_traverse. It has been almost 3 years since I looked at this issue, and I do not remember how it is supposed to work, but that looks like it is designed for your use case.

If you cannot get that to work, then you might need something kludgy. Like use a static value for data-entity-uuid, then have a separate step in the process pipeline to replace that with the correct value.

I think I created this issue, but I did not attend the meeting.

benjifisher → created an issue.

Only two people joined the Zoom meeting this week, so we decided to cancel the meeting.

I am adding credit for the contributors mentioned in Comment #70.

benjifisher → created an issue.

benjifisher → created an issue.

There is a merge conflict because 📌 Convert test annotations to attributes in core's Unit tests Active changed the @dataProvider annotation to an attribute. I resolved the conflict by removing both the test and the data provider, as in the earlier version of the MR.

The merge conflict is because this issue added a function to NodeHooks1.php, which was removed in 📌 Remove NodeHooks1 Active . Following the recommendation in 📌 [meta] Clean up hook classes in core Active (and the text file attached to that issue) I moved the implementation of field_ui_preconfigured_options_alter() to a new class, NodeFieldHooks.

I also moved the implementations of the same hook in the taxonomy and user modules to new classes (TaxonomyFieldHooks and UserFieldHooks). When the meta issue reaches those two modules, a little bit of the work will already be done.

Another fix would be to move the hook above the opening of the group:

/**
 * @addtogroup field_purge
 * @{
 */

In fact, it would be a good idea to reorganize all the hooks in field.api.php, but that would not be a Novice task. In the Proposed resolution, I recommend moving the hook after the close of the group for two reasons:

1. It is the minimal change to fix the problem.
2. If another issue adds a new hook to the end of the file, then that new hook will be outside the group.

benjifisher → created an issue.

My understanding is that config schemas are for config entity types. In theory, we could have a config schema for migrate_plus.migration.*, and in theory we could generate a config form from that schema. In practice, I do not see how we can get that to work unless we have some way for process plugins to specify their config forms.

Background: the main part of any migration (core plugin or migrate_plus config entity) is the process section. This consists of several process pipelines. Many pipelines are single steps (like assign a source value to a destination value) but some pipelines are long chains of process plugins, each with its own configuration.

Source, process, and destination plugins are not config entities, so config schemas do not apply directly. In Comment #3, I suggested an approach to getting config schemas to apply. But that is still in the ideation phase, a few steps before vaporware.

benjifisher → created an issue.

I am copying the list of participants from the Security Team's private notes.

benjifisher → created an issue.

@thejimbirch: Thanks for opening this issue.

@jurgenhaas: There are only a few source and destination plugins, but there are a lot of process plugins. These are PHP classes, generally extending Drupal\migrate\ProcessPluginBase. So I am not sure what config schema you mean.

When I called it "a long-term project", I was thinking of the old issue 📌 Create a trait and base class to implement \Drupal\Component\Plugin\ConfigurableInterface Needs work , which was Fixed 2 or 3 months ago (2025-06-14). I think the next step is to expand Drupal\Component\Plugin\ConfigurableInterface, or add a related interface, with some sort of form-builder method. Then configurable plugins would have a consistent way to provide their configuration forms.

What that new, public method would be, and how it is implemented, is up for discussion. Maybe it would involve a config schema, at least as one option. Or maybe it would involve a config entity type, and the config schema would declare that it handles that type.

That is why I describe this issue as providing a proof of concept. For now, then suggested module is responsible for providing config forms for a limited number of plugins.

The suggested module would not be a candidate for Drupal core. Since it relies on migrations defined by config entities, it has to depend on the migrate_plus module. I suggested using the csv source plugin for the proof of concept (and for making something that is actually useful). That comes from another contrib module, migrate_source_csv.

benjifisher → created an issue.

There was no meeting on August 14, so I am repurposing this issue for the August 28 meeting.

greggles → credited benjifisher → .

mcdruid → credited benjifisher → .

@camhoward:

I guess we agree on most things, but not all.

Instead of adding weights everywhere, we get the same result if we simply revert the change from #2219393. We go back to the order of the primary tabs before Drupal 11.2, and primary tabs are listed by weight and then the order in which they are defined. That is what I did in MR !12792.

I do not see the advantage of sorting alphabetically and then adding weights to everything so that the alphabetical sort never applies.

Clarification: sorting by weight is always primary, before and after #2219393. The only question is whether the secondary sort should be alphabetical or order in which they are defined.

@godotislate:

I think the BC problems are easier to handle than you suggested in #36.

A boolean field effectively has 3 possible values: TRUE, FALSE, or NULL. When rendering the block and inspecting the value of the new config item, treat NULL as TRUE.

Someone (@alexpott) once commented that ... ?? TRUE had a bad "code sniff" or something like that. So probably add a code comment, explaining that this is for BC.

We looked at this issue again at 📌 Drupal Usability Meeting 2025-08-15 Active . (Present: benjifisher, rkoller, simohell.)

We still think that something positive ("Add a CSS class") is clearer than something negative ("Be the same on all pages"). But you make a good point about not mentioning the "active-trail class" in the UI. Can we compromise on something less specific, like one of these?

• Add a CSS class to parent links
• Highlight the ancestors of the active menu link with a CSS class
• Add a CSS class to ancestors of the current page
• Add a CSS class to links leading to [or above] the current page

The goal is that the typical user will have to read the help text (description) once, and then the label should be enough when they return to the configuration form. So the label does not have to say too much. On the other hand, the help text should explain

• This option has an effect when the block is shown on a page listed in the block.
• It applies to all "parents" or "ancestors" of the current page that are shown in the block.
• Adding the CSS class has performance implications.

It took two tries, but I added the assertions for the button text, before and after toggling.

Unless I totally misunderstand the nth-child selector, my mistake was to look for the second item in the admin menu after installing the Standard profile. That is Structure. I am not sure which profile the Nightwatch tests use, but it seems that the second item is Configuration.

Back to NR.

benjifisher → created an issue.

I am adding issue credits based on the Security Team's private notes.

benjifisher → created an issue.

How is it easier to use the \PASSWORD_* constants? Using drush php:

> password_algos()
= [
    "2y",
    "argon2i",
    "argon2id",
  ]

I think we should let people set a hashing algorithm in services.yml.

Have we considered what to do if the service parameter is invalid? Do we fall back to PASSWORD_DEFAULT? Is there a requirements check (for the status page)?

In https://git.drupalcode.org/security/24-drupal-security/-/issues/1, we discussed a problem in the 11.x branch that did not exist in any released version of Drupal core. The issues that led to that problem were reverted, so the problem will never be part of a release.

I know that someone reading that can search for reverted issues and figure out where the problem was, but the problem has been fixed so I am not worried about mentioning it on this public issue.

The question for this issue is what label to attach when closing the private issue. I chose Closed:public, but I would have liked the option Closed:outdated. Should we add such a label?

@andypost:

What transition do you mean?

Passwords should be rehashed automatically when users log in with their passwords. The Q&A for the Password Compatiblity module → apply to this situation, too.

From #24 here:

I've manually tested that changing the hashing algorithm with existing users successfully rehashes their password in the new algorithm when they next login.

@znerol:

Should we add a config and a proper UI to make these requirement checks more actionable?

I have been thinking that this is something that would be managed through settings.yml, not the UI. In fact, I like the idea of relying on something in the filesystem. Otherwise, XSS can be exploited to change the password hashing algorithm.

If you still want to provide a configuration form, then let's have a followup issue for further discussion.

@berdir:

That is a good point.

Both the commerce module and the physical module (Physical Fields) define their own field categories and put their fields under those categories. I disagree with that decision, but I do not know if I can persuade them to change.

During the usability meeting, we thought that many users would not think that "number plus unit" would be under the Number category, so we wanted to give an example to suggest that. The core fields are suitable for simple uses: in addition to the numeric value stored per field instance, the configuration can store a prefix or suffix to indicate a type of currency or a physical unit.

If we do not use price nor physical unit, in order to avoid confusion with those two contrib modules, then what else can we use as an example? I do not think that weight gets the point across. Do you have any other suggestions?