Problem/Motivation
The annotation in core/modules/block_content/src/Entity/BlockContentType.php
looks like this:
* @ConfigEntityType(
* id = "block_content_type",
* label = @Translation("Custom block type"),
* label_collection = @Translation("Custom block types"),
* label_singular = @Translation("custom block type"),
* label_plural = @Translation("custom block types"),
* label_count = @PluralTranslation(
* singular = "@count custom block type",
* plural = "@count custom block types",
* ),
* label_collection = @Translation("Custom block library"),
Proposed resolution
As you can see, label_collection
is defined TWICE.
- The first
label_collection
was added as part of an older issue to add entity labels: #2702683: Add plural labels to entity types - The second
label_collection
was committed two months earlier than that, by: #2919890: Make BlockContentType use a route provider instead of hard-coded routes
I checked all content and config entities defined by core, for duplicate use of label
, label_collection
, label_singular
, label_plural
, or label_count
, and this is the only instance of duplicate labels I found.
This problem exists in D8.8, D8.9, D9.0, and D9.1.
The patch removes the second use of label_collection
because the text is not consistent with the other labels.
Remaining tasks
None
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
None
Comment | File | Size | Author |
---|---|---|---|
#8 | custom-block-library.png | 42.51 KB | jungle |
#8 | custom-block-types.png | 44.27 KB | jungle |
#7 | 3137430-7-label-collection.patch | 973 bytes | TR |
#2 | 3137430-2-label-collection.patch | 587 bytes | TR |
Comments
Comment #2
TR CreditAttribution: TR commentedComment #3
TR CreditAttribution: TR commentedComment #4
TR CreditAttribution: TR commentedComment #5
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae as a volunteer and commented+1 to commit this patch. RTBC.
Comment #6
alexpottI'm not sure about removing the second one. It changes the title on admin/structure/block/block-content/types and the result of calling \Drupal::entityTypeManager()->getDefinition('block_content_type')->getCollectionLabel();
The no change version of this change is to remove the first one.
Comment #7
TR CreditAttribution: TR commentedYes, it does change the title, but why is that bad?
This is not a breaking string change - BOTH of the strings are currently defined and exist in the translation table. We're removing one of the strings with this patch but we're not changing any remaining strings.
Right now the other labels on the entity are:
So to me it makes sense to use:
* label_collection = @Translation("Custom block types"),
But if it concerns you, here's an alternate patch that instead uses:
* label_collection = @Translation("Custom block library"),
Comment #8
jungleUsing
Custom block types
is better thanCustom block library
to me. But from my understanding, for history reasons, keeping it asCustom block library
is better, or end users might get confused and try to findCustom block library
still.Just like the well-known typo HTTP referer (a misspelling of referrer).
So #7 is RTBC.
Comment #9
jungle#3116502: Wrong field_ui_base_route or duplicate field_ui_base_route in badge entity is a similar one which filed by me. I am pondering that is it worthy to write a test for this to avoid duplicate keys in annotations of config entities, annotation-based plugins, as this is a quick fix. I won't tag "Needs test", so let's do it in a separate issue if we agree on this.
Comment #10
alexpottCommitted and pushed 2dbb3e450a to 9.1.x and ad66016aac to 9.0.x and 48945fd26c to 8.9.x. Thanks!
Now we're not changing any UI strings this can be backported all the way to 8.9.x
Comment #14
alexpott@jungle +1 to opening a coder issue to check for duplicate annotation keys... it might be worth looking to see and any third party sniffs cover annotations.
Comment #15
jungle@alexpott, thanks for committing! An issue opened to coder.