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

Form checkValidity() sometimes returns true when form fields are invalid #2256

Open
kevinkace opened this issue Oct 22, 2018 · 4 comments
Open
Labels
Area: Core For anything dealing with Mithril core itself Type: Bug For bugs and any other unexpected breakage

Comments

@kevinkace
Copy link
Contributor

When using m.withAttr() to maintain an input value, the form's checkValidity() method sometimes returns true when form fields are not valid. This happens on the second and subsequent submissions.

Expected Behavior

checkValidity() should always return false if any fields within the form aren't valid, eg: https://codepen.io/kevinkace/pen/rqKroe?editors=1011

Current Behavior

checkValidity() returns true when a field contains an invalid value, on the second and subsequent submits. eg: https://codepen.io/kevinkace/pen/XxYBoK?editors=0011

Steps to Reproduce (for bugs)

https://codepen.io/kevinkace/pen/XxYBoK?editors=0011

  1. use m.withAttr() to set the value of an input with a minlength, eg:
m("input", {
  minlength : 4,
  value : vnode.state.value,
  oninput : m.withAttr("value", function(value) {
    vnode.state.value = value;
  }
})
  1. add an onsubmit handler to the form element, eg:
m("form", {
  oncreate : function(formVnode) {
    vnode.state.formDom = formVnode.dom;
  },
  onsubmit : function(e) {
    e.preventDefault();
    vnode.state.formDom.checkValidity();
  },
  // form elements...
)
  1. add a button to the form, eg m("button", "submit")
  2. add 3 characters to the input
  3. click the button, checkValidity() should return false
  4. click the button, checkValidity() now returns true

Context

I'm trying to used HTML form validation for form submition.

Your Environment

  • Version used: 1.1.6
  • Browser Name and version: Chrome 70, Firefox Quantum 62
  • Operating System and version (desktop or mobile): Windows 10
  • Link to your project: https://github.com/kevinkace/lyrite/tree/firebase (code of issue isn't yet submitted)
@dead-claudia
Copy link
Member

dead-claudia commented Oct 22, 2018

So here's what I see: when you click the button[type=submit], it no longer sees the input as the active element, and thus doesn't think to check that the values are equal. If we were to remove the $doc.activeElement === vnode.dom check here, I'd suspect that'd likely solve your problem. FWIW, we already had to do that to work around a Chrome bug where changing it to the same value erroneously resets the cursor, so this is actually reducing the amount of work we're doing.

@dead-claudia
Copy link
Member

Personally, I'd like to double-check whether document.activeElement really needs checked here as well.

The check here and here is required to work around this IE/old Edge bug and an IE9 bug referenced here, and probably should have a comment added for it at least.


But in summary, it's just a case where we're unnecessarily checking document.activeElement, and I suspect there might be others, too.

@fuzetsu
Copy link
Contributor

fuzetsu commented Oct 22, 2018

@isiahmeadows I agree with your assessment. The document.activeElement in setAttr seems unnecessary.

removing document.activeElement seems to work

@StephanHoyer
Copy link
Member

StephanHoyer commented Feb 21, 2022

it's still an issue

@dead-claudia dead-claudia moved this to Low priority in Triage/bugs Sep 2, 2024
@dead-claudia dead-claudia added the Area: Core For anything dealing with Mithril core itself label Sep 2, 2024
@dead-claudia dead-claudia mentioned this issue Oct 13, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Core For anything dealing with Mithril core itself Type: Bug For bugs and any other unexpected breakage
Projects
Status: Low priority
Development

No branches or pull requests

4 participants