Closed (fixed)
Project:
MaxLength
Version:
2.0.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
17 Oct 2017 at 04:41 UTC
Updated:
12 Aug 2022 at 08:15 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
joshua.boltz commentedComment #3
dunot commentedOn core 8.6.7 and Maxlength 8.x-1.0-beta2 or maxlength-8.x-1.x-dev.tar #2 makes
but after logout/login works perfect, thank you!
Comment #4
joshua.boltz commentedComment #5
joshua.boltz commentedComment #6
showardclark commentedInstalled and tested on core 8.8.2 and Maxlength 8.x-1.0-beta5. Working for my use case.
Thanks!
Comment #7
publishing future commentedPatch in #2 works fine for me with Drupal 8.8.4 and Maxlength 8.x-1.0-beta5 as well.
Comment #8
hydra commentedThis seems indeed to be an unwanted behaviour, since there is a checkbox to enforce maxlength...
Rerolled the patch for 1.0.0-rc1
Comment #9
renrhaf+1 RTBC for patch #8 on version 8.x-1.0.0-rc1
Comment #10
renrhafThe patch in #8 did not meet our requirements because having both soft (data-maxlength change in #8) and hard limits (with maxlength HTML attribute) is not possible. Here is a new patch adding a new widget option, a checkbox named "Use maxlength HTML property" that can be checked only when "Force truncate" itself is checked. Also added some code for field "summary" text support.
Check screenshot : https://res.cloudinary.com/renrhaf/image/upload/v1606860703/maxlength-29...
Comment #11
renrhafSmall fix for translating the countdown message.
Comment #12
renrhafOops as per https://www.drupal.org/project/maxlength/issues/2823995#comment-12155956, we should leverage config translation for those things. Reverting my change for translation.
Comment #13
renrhafMoving code around so that the maxlength attribute can be enforced even if we do not want to truncate the text
Comment #14
ricovandevin commentedPatch works. I guess we should also use data attributes for the other attributes specific to this module. But that is a different story.
Comment #15
DrupalHack commentedWhen the html attribute is not used, maxlength.js is still trying to use it.
A text field with a 70 character limit, not using the maxlength attribute displays as 255.
A text area with a 160 char limit, not using the maxlength attribute displays as Nan.
Attached is the latest patch from #13, but it updates maxlength.js to only select the
data-maxlengthattribute, instead of themaxlengthattribute. This way, when the maxlength attribute is used, the browser will enforce the field's character limit on its own. But either way, the maxlength count will be displayed correctly using thedata-maxlengthvalue.Comment #16
cedeweyComment #17
cedeweyI feel like I am missing something obvious here, but the way that the module currently works is that there is an option for Force truncate. If that is enabled then the author cannot enter text beyond the limit. However, if Force truncate is disabled, then the maxlength is indeed just a recommendation and the counter displays the negative number.
Isn't the module already behaving as expected?
Comment #18
DrupalHack commentedIf the maxlength attribute is present, the browser itself is limiting the text entered into the field, so regardless of the Force Truncate setting, an editor is still given a hard character limit. This may be something that behaves differently for plaintext fields than for text areas. My use case was for plaintext fields.
Comment #19
cedeweyOk, yes I am experiencing this hard limit on plain text field, but not on rich text fields. Thank you for explaining that for me.
I tested your patch and it does indeed resolve the issue.
We are rethinking the general workflow of Maxlength to improve overall usability. Whatever comes from that will affect this issue as well. I'll create a ticket for that so that we're thinking out loud in the public together as a community.
Something to discuss is whether to ever rely on the HTML maxlength property. I realize if we don't then JavaScript is required, but it seems like that might be an ok requirement to have since this is a module for content authors and not having JS running seems like an edge case.
I'm very curious to hear what others think about this though.
Comment #20
cedeweyComment #21
cedeweyI did some further testing and this patch is working for long plain text fields. However, for short plain text fields even if the "Use "maxlength" HTML property" checkbox us unchecked, I'm still prevented from typing beyond the limit defined in the "Maximum length" field setting.
Comment #22
dalinPatch no longer applies against the current release. Which is maybe a good thing since it sounds like the module is going in a different direction in #3259248: Improve the user experience of configuring a Maxlength limit. and applying this patch would leave you with a forked config schema.
Comment #23
cedeweyAfter further discussion we've decided to move ahead with this fix. There's been some misunderstanding as to the intent of this issue. To clarify I've updated the issue title and summary.
We are strongly leaning towards using the maxlength-data attribute exclusively and dropping the use of the maxlength HTML attribute. That way soft limits will work on plain text fields.
The one use case I can see for using the HTML attribute is for site authors not running JS, but that seems like a real edge case. If anyone else from the community has opinions on this please share them. A possible resolution for those users is a separated module called something like Simple Maxlength that simply has a configuration option to set a value for the maxlength HTML attribute.
Comment #26
cedeweyGreat work everyone on this ticket. We altered the patch in #15 to remove the option to use the HTML attribute based on the values of keeping the module simple and geared towards site builders. There was too much of an edge case for keeping that.
We reviewed this together so went ahead and deployed it to the main branch.
Comment #29
stefan.kornIs it intended that for using a hard limit on string_textfield, now I need to set the limit in the core field settings? I cannot enforce a hard limit on an string_textfield with the maxlength settings alone anymore?
Imho this cannot be intended this way. The setting in the core field settings is a valid for all fields instances. So you cannot have different hard limit on the same field in multiple instances?
Should the "Hard limit" setting (maxlength_js_enforce) be made available for string textfields too?
Comment #30
stefan.kornTried a patch that would allow to set a hard limit on string_textfield with maxlength by setting (overriding) the HTML maxlength attribute in that case.
Maybe not in line with
but I really think this is something which is expected to be feature of maxlength (as it was before).
That said: I think there is even a BC break here. If you go from 1.x to 2.x, you might expect that the limit you set on string_textfields is still a hard limit (not depending on the field storage max length setting). But with 2.x this will now be a soft limit. So maybe having "hard limit" as default setting for string_textfield would be a measure to avoid this BC break?
Comment #32
cedeweyHi @stefan.korn,
Thanks for bringing this up. This is a known issue and the long term fix is #3259425: Configure limit method on plain text fields. Would you be willing to rework your patch for that issue? It should indeed use the maxlength-data attribute.
As for which limit method to be the default one, you make a good point that since some of the field types previously only used the hard limit method, that might be a better user experience. We did however introduce the 2.0 release exactly so that we could introduce breaking changes if need be.
I've created a feature request where people can discuss this further. #3291290: Set the default limit method to hard limit
Comment #33
stefan.korn@cedewey: Thanks for explaining. I am sorry, I have not had the time to maybe rework the patch in line with #3259425: Configure limit method on plain text fields. I will try to find some time for this. In the meantime I am uploading a new version of patch from #30 that works against latest maxlength release.