Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing textarea attributes #440

Merged
merged 8 commits into from
May 11, 2022
Merged

Add missing textarea attributes #440

merged 8 commits into from
May 11, 2022

Conversation

evert
Copy link
Collaborator

@evert evert commented May 11, 2022

This is the same PR as nodejs/node#439, but applied to the stable version-7.x branch

@evert evert requested a review from dayre May 11, 2022 03:50
@evert evert self-assigned this May 11, 2022
@evert evert added the bug label May 11, 2022
@evert evert enabled auto-merge May 11, 2022 03:50
@evert
Copy link
Collaborator Author

evert commented May 11, 2022

@dayre I must have missed this the first time, but why were these attributes added to HiddenField as well?

@evert
Copy link
Collaborator Author

evert commented May 11, 2022

@dayre Just in case you're interested, i think the failing tests were due to a new Node 18 bug: nodejs/undici#1433

@dayre
Copy link
Collaborator

dayre commented May 11, 2022

Re the hidden field addtions... my mistake... I was referencing the RFC for minlength/maxlength attributes being used for input and textarea fields. I didn't look at specifically at the section on inputs for type=hidden which does say they must not be specified and do not apply: https://www.w3.org/TR/2014/CR-html5-20140429/forms.html#hidden-state-(type=hidden)

dayre added 2 commits May 11, 2022 08:52
Removed minLength/maxLength attributes for input type=hidden per RFC
Removed minlength/maxlength from input type=hidden per RFC
@evert evert disabled auto-merge May 11, 2022 15:56
@evert evert merged commit 0cdf068 into version-7.x May 11, 2022
@evert evert deleted the textarea-attributes branch May 11, 2022 15:56
@evert
Copy link
Collaborator Author

evert commented May 11, 2022

lgtm!

Copy link
Collaborator

@dayre dayre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants