- Status changed to RTBC
over 1 year ago 3:18pm 13 February 2023 - ๐ฎ๐ณIndia ameymudras
Tested on 10.1.x and following is my observation
1. The issue summary is clear and explains the overall problem
2. Testing steps have been provided and was able to reproduce the issue
3. Patch #24 applies cleanly and the non-ASCII config entity sort correctly
4. Tests have been included and passes for #24
5. Did code review and no issues were identifiedMarking this as RTBC, not including additional screenshots. Already provided in #27
- Status changed to Needs review
over 1 year ago 12:14pm 21 March 2023 - ๐ฌ๐งUnited Kingdom longwave UK
+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php @@ -226,14 +226,22 @@ public function createDuplicate() { + $language_id = \Drupal::service('language_manager') ... + $a_label = \Drupal::service('transliteration')->transliterate( ... + $b_label = \Drupal::service('transliteration')->transliterate(
A bit concerned about calling all these services inside the sort callback, because it feels like this is not going to be very performant in a case when there are hundreds of items or more to sort.
We could at least extract the transliteration service to a variable, and also skip transliterating the empty string if it is falsy? Is there a way of injecting at least the language ID, given that it never changes, or the transliteration service itself?
Also, do we have the same concerns about sorting in different languages as was raised in #3262017-90: Country list is not correctly sorted when it's localized with accents (e.g. German, Turkish) โ ? Is transliterating the right thing to do in all cases?
- Status changed to Needs work
about 1 year ago 9:30pm 2 April 2023 - Status changed to Needs review
about 1 year ago 9:38pm 2 April 2023 - ๐ฉ๐ชGermany sleitner
@longwave : I remove transliteration and added the
Collator
like in ๐ Country list is not correctly sorted when it's localized with accents (e.g. German, Turkish) Postponed . - ๐ฉ๐ชGermany sleitner
To prevent MethodArgumentValueNotImplemented exception in Collator, language_id en is used if intl extension is not installed.
- ๐ฌ๐งUnited Kingdom longwave UK
The new polyfill and intl extension have the same considerations as ๐ Country list is not correctly sorted when it's localized with accents (e.g. German, Turkish) Postponed so this will need to wait for the decision over there first.
- last update
about 1 year ago 29,277 pass, 2 fail The last submitted patch, 38: 2265487-38.patch, failed testing. View results โ
- last update
about 1 year ago 29,300 pass - Status changed to Postponed
about 1 year ago 6:57pm 6 May 2023 - ๐บ๐ธUnited States smustgrave
Postponing on ๐ Country list is not correctly sorted when it's localized with accents (e.g. German, Turkish) Postponed .
- ๐บ๐ธUnited States smustgrave
Also posted to the #needs-review-queue-initative slack channel so hopefully a framework manager can take a look at that one.
- last update
about 1 year ago Patch Failed to Apply - Status changed to Needs work
4 months ago 9:27pm 2 March 2024 - ๐ฉ๐ชGermany sleitner
Unpostpone it. ๐ Country list is not correctly sorted when it's localized with accents (e.g. German, Turkish) Postponed is postponed because nobody wants to break the API.
- Merge request !6869Issue #2265487: ConfigEntity based lists with items containing non-ascii... โ (Open) created by sleitner
- Status changed to Needs review
4 months ago 12:00am 3 March 2024 - Status changed to Needs work
4 months ago 7:23pm 5 March 2024 - ๐บ๐ธUnited States smustgrave
Can the issue summary be updated to use the standard template please.
Hiding all patches for clarity as fix is in MR now.
Will need framework manager review for the package being added.
- Status changed to Needs review
about 2 months ago 3:56pm 13 May 2024 - Status changed to Needs work
about 1 month ago 10:17pm 19 May 2024 The Needs Review Queue Bot โ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide โ to find step-by-step guides for working with issues.
- Status changed to Needs review
23 days ago 7:02am 5 June 2024 - Status changed to Needs work
18 days ago 1:47pm 10 June 2024 - ๐บ๐ธUnited States smustgrave
Took a look at the MR before pinging a framework manager and seems we are updating several packages that seem unrelated to this. Can those be reverted please.
- Status changed to Needs review
17 days ago 7:14pm 10 June 2024 - Status changed to Needs work
13 days ago 10:49am 15 June 2024 The Needs Review Queue Bot โ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide โ to find step-by-step guides for working with issues.
- Status changed to Needs review
12 days ago 7:04pm 15 June 2024 - Status changed to RTBC
4 days ago 1:50pm 24 June 2024 - ๐บ๐ธUnited States smustgrave
I can't make the call about the new package but the MR does fix the problem described in the issue summary. Moving to RTBC to put in front of the framework managers.