- Issue created by @Anybody
- last update
over 1 year ago 29,388 pass - @anybody opened merge request.
- last update
over 1 year ago 29,387 pass, 2 fail - @anybody opened merge request.
- last update
over 1 year ago 29,387 pass, 1 fail - last update
over 1 year ago 29,365 pass, 2 fail - @anybody opened merge request.
- @anybody opened merge request.
- 🇩🇪Germany Anybody Porta Westfalica
Implemented the 4 options a - d as example MR's. I'd tend to choose a or b and I think the limit of 128 characters never should have been introduced after Drupal 7 with configuration.
- Status changed to Needs review
over 1 year ago 7:35am 17 May 2023 - 🇩🇪Germany Anybody Porta Westfalica
As next step it should be discussed which way to go with which risks, eventually BC's (= core target version) etc. The MR's are just examples to show possible ways. Would be nice to have core maintainers feedback here.
- Status changed to RTBC
over 1 year ago 7:13am 22 May 2023 - 🇩🇪Germany Grevil
I'd agree with either solution a) or b), where setting the limit to 256 would be more explicit, as you wouldn't usually need a higher char count for labels etc. anyway. But removing the maxlength entirely could help with the remaining 1% needing a higher char count.
So if it wouldn't have any negative side effects, I'd vote for solution a), otherwise b).
- last update
over 1 year ago 29,388 pass - last update
over 1 year ago 29,387 pass, 2 fail - last update
over 1 year ago 29,387 pass, 1 fail - last update
over 1 year ago 29,386 pass, 3 fail - last update
over 1 year ago 29,388 pass - last update
over 1 year ago 29,388 pass Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened → , as Drupal.org infrastructure cannot currently fully support a branch named
main
. New developments and disruptive changes should now be targeted for the11.x
branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule → and the Allowed changes during the Drupal core release cycle → .- last update
over 1 year ago 29,395 pass - last update
over 1 year ago 29,396 pass - last update
over 1 year ago 29,399 pass - last update
over 1 year ago 29,399 pass - last update
over 1 year ago 29,400 pass - last update
over 1 year ago 29,409 pass - last update
over 1 year ago 29,414 pass - last update
over 1 year ago 29,418 pass - last update
over 1 year ago 29,420 pass - last update
over 1 year ago 29,420 pass - last update
over 1 year ago 29,426 pass - last update
over 1 year ago 29,429 pass - last update
over 1 year ago 29,430 pass - last update
over 1 year ago 29,430 pass - last update
over 1 year ago 29,436 pass - 🇬🇧United Kingdom longwave UK
Out of interest I looked through git history and found that we set maxlength to 128 for textfields in #35644: Change defaults on form elements to most common values → - previously it was an arbitrary value of 70 instead.
I am not sure what the best option is here. I don't really like any of them except the simplest one: remove the default maxlength entirely - but I also share the BC concerns. There is definitely a risk of introducing bugs if we just drop the length, because of things like database column lengths.
We could (in theory) deprecate it by detecting when it's not set, warning users that #maxlength default will change in the next major, etc, but is it worth it? I am not entirely sure of the benefit of changing it now given that form API elements can just override this value if they need to.
Adding framework manager review tag to get more visibility on this.
- 🇩🇪Germany Anybody Porta Westfalica
Thanks for the feedback @longwave. I shouldn't repeat myself, as I already wrote those points into the issue summary, but as short reply to your comment
- I think this limitation is simply unexpected from a framework. When using an input field without providing a #maxlength you may not even think about (or try) there's such a limit. That's my a framework shouldn't do it in my oppinion.
- Currently, I only see a potential risk for database schema, config schema doesn't seem to have (any) limits. If we can split these and apply a limit only to database forms or protect them in any other way, that would unblock this limitation
- It leads to real issues in forms like the core-provided translation forms, where the input is shorter than the original field, see 🐛 Cookies Service Input fields limit text length too short Fixed
- I think it mainly has historic reasons, and we should take the steps to remove it. If not possible, we should at least increase the limit further to 256 characters so it's even more unlikely that people run into it, but I think it still wouldn't be the right way. But better :)
- last update
over 1 year ago 29,436 pass - last update
over 1 year ago 29,436 pass - last update
over 1 year ago 29,441 pass - last update
over 1 year ago 29,444 pass 11:10 7:32 Running- last update
over 1 year ago 29,443 pass - last update
over 1 year ago 29,439 pass - last update
over 1 year ago 29,439 pass - last update
over 1 year ago 29,441 pass - 🇫🇷France duaelfr Montpellier, France
I faced that issue in an views exposed filter on an entity reference field using the autocomplete widget.
One of my users tried to search for a referenced entity which title is 138 chars long (+ double quotes and ID between parenthesis). The autocomplete works well and the field is filled when selecting the right result but after submitting the form, they gets an error and no results. - last update
over 1 year ago 29,444 pass - last update
over 1 year ago 29,444 pass - 🇬🇧United Kingdom catch
Thinking about the deprecation, I'm not sure we can do that, unless we want to force everyone to set a maxlength, otherwise there's no way to remove the deprecation message for modules that don't want it set.
There is definitely a risk of introducing bugs if we just drop the length, because of things like database column sizes being overflowed where previously this was not possible.
This is true but I feel like for entities this would already be a problem due to JSON:API. The database will either truncate the value or throw an exception, the exception could be jarring, but it would only be for someone who previously would not have successfully submitted that form with the same values.
I think this is probably OK to do in a minor release, but with quite a loud change record/release note.
It's also the sort of thing we could make 'major version only', generally we've avoided that as much as possible, but this might be a case where that would work.
- last update
over 1 year ago 29,446 pass - last update
over 1 year ago 29,445 pass, 1 fail - last update
over 1 year ago 29,446 pass - last update
over 1 year ago 29,446 pass - last update
over 1 year ago 29,446 pass - last update
over 1 year ago 29,445 pass, 1 fail - last update
over 1 year ago 29,446 pass - last update
over 1 year ago 29,446 pass - 🇩🇪Germany Anybody Porta Westfalica
Ok we shouldn't try to change the MR to 11.x ;D
This should be just fine and patchable ... - last update
over 1 year ago 29,446 pass - Open on Drupal.org →Environment: PHP 8.2 & MySQL 8last update
over 1 year ago Waiting for branch to pass - last update
over 1 year ago 29,446 pass - last update
over 1 year ago 29,450 pass - last update
over 1 year ago 29,453 pass - last update
over 1 year ago 29,454 pass - last update
over 1 year ago 29,455 pass - last update
over 1 year ago 29,456 pass - last update
over 1 year ago 29,457 pass - last update
over 1 year ago 29,458 pass - last update
over 1 year ago 29,458 pass - last update
over 1 year ago 29,459 pass - last update
over 1 year ago 29,465 pass - last update
over 1 year ago 29,465 pass - last update
over 1 year ago 29,465 pass - Status changed to Needs review
over 1 year ago 3:56am 15 August 2023 - 🇳🇿New Zealand quietone
I am doing triage on the core RTBC queue → .
I read the issue summary and the most importantly the proposed resolution is a set of options. I then read the comments to figure that out. The comment that set this to RTBC supported 2 of the options. Since that is not clear I am tagging for an IS update and setting to NW.
In #15 catch asked for a CR. I am adding that tag. Of course, that shouldn't be made until this has a definite resolution. He also considered that this might be done in a major release. Therefor, I am tagging for RM review as well.
I am setting to NR for discussing and selecting the option to implement.
- Status changed to Needs work
over 1 year ago 5:31pm 11 October 2023 - 🇺🇸United States smustgrave
For the issue summary update and change record.
- last update
about 1 year ago 29,675 pass, 1 fail - 🇩🇪Germany Anybody Porta Westfalica
Marked the approaches we probably won't take as "Draft". Maybe useful for a follow-up or discussions. Feel free to close otherwise.
- Status changed to RTBC
about 1 year ago 10:47am 26 October 2023 - 🇩🇪Germany Anybody Porta Westfalica
Issue summary updated. Waiting for Needs framework manager review, Needs release manager review now!
- Status changed to Needs work
about 1 year ago 11:47am 26 October 2023 - Status changed to RTBC
about 1 year ago 12:00pm 26 October 2023 - 🇩🇪Germany Grevil
OK, the last failing test is simply an unrelated racing condition problem. I don't think, this should be fixed in this issue, as it is completely unrelated.
- Status changed to Needs work
about 1 year ago 1:39pm 26 October 2023 - 🇩🇪Germany Grevil
Setting this back to "Needs work" as I don't think any core maintainer would commit an MR containing failed tests. Maybe someone can help with 🐛 testCreateViewWizard fails, when removing the "#maxlength" property of the textfield form element Needs work . I have no idea, what makes the test fail and can not reproduce it through manual testing.
- last update
about 1 year ago 29,676 pass - 🇩🇪Germany Grevil
Alright, back to needs review! Special thanks to @lendude, who provided the "_populate" fix on Slack! Thanks again! 👍
Please credit him manually! - Status changed to Needs review
about 1 year ago 2:30pm 26 October 2023 - 🇩🇪Germany Grevil
Pipeline is green! https://git.drupalcode.org/issue/drupal-3331028/-/pipelines/39305
- Status changed to RTBC
about 1 year ago 3:07pm 26 October 2023 - last update
about 1 year ago 29,676 pass - last update
about 1 year ago 29,678 pass - last update
about 1 year ago 29,678 pass - last update
about 1 year ago 29,678 pass - last update
about 1 year ago 29,678 pass - last update
about 1 year ago 29,678 pass - last update
about 1 year ago 29,678 pass - last update
about 1 year ago 29,682 pass - last update
about 1 year ago 29,683 pass - last update
about 1 year ago 29,686 pass - last update
about 1 year ago 29,686 pass - last update
about 1 year ago 29,688 pass 56:02 51:34 Running- last update
about 1 year ago 29,721 pass - last update
about 1 year ago 29,721 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,721 pass, 2 fail - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - last update
about 1 year ago 29,722 pass - Status changed to Needs review
about 1 year ago 4:55pm 8 January 2024 - 🇬🇧United Kingdom catch
Are there any existing forms in core (entity labels? Machine names?) where we might be relying on the default 128 #maxlength and need to add an explicit value in those forms?
- 🇩🇪Germany Anybody Porta Westfalica
@catch: Any ideas how we can be sure about that?
Looking at this: https://git.drupalcode.org/search?search=%23maxlength&nav_source=navbar&...
it seems there are at least many cases where peolple added the value, where it was needed!An important example you made: Machine names:
https://git.drupalcode.org/issue/drupal-3331028/-/blob/3331028-increase-...So I think we should ensure this is safe wherever possible, but on the other hand things won't get better, if we wait longer to "fix" this. Happy to help if anyone has further ideas how to be even more safe!
- 🇬🇧United Kingdom catch
@Anybody can we look at core usages of #type 'textfield' where it doesn't have #maxlength?
- 🇺🇸United States smustgrave
Following up @Anybody if you had a chance to look at #37.
- 🇩🇪Germany Anybody Porta Westfalica
@smustgrave thanks for the ping, I didn't forget this, but currently can't take the time. Open for anyone, sorry.
- Status changed to Needs work
11 months ago 10:07am 22 February 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.
- 🇺🇸United States mile23 Seattle, WA
I did a search in core (granted, it was 10.2), for
'#type' => 'textfield',
, and stopped counting at 10 when I found files that rely on the default value.Also, it seems like relying on the default value is kind of OK, which is why it's there.
Config offers a schema constraint of
Length.max
, so it seems like a config form would use that value if it's present, only using the form element default if there was no such constraint. Ideally this would happen in some automatic or semi-automatic way to make it easy to build config forms. Maybe as a method onConfigFormBase
that does magic for you.It seems like there would be some other issue where this is happening, but I don't see it, other than in initiatives to make sure all config has constraints: 🌱 [meta] Add constraints to all config entity types Active It also seems like it would slot right into this CR's purpose: https://www.drupal.org/node/3373502 →
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.