mithril.js Form checkValidity() sometimes returns true when form fields are invalid

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

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;
  }
})

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...
)

add a button to the form, eg m("button", "submit")
add 3 characters to the input
click the button, checkValidity() should return false
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)

Oct 22 '18 05:10 kevinkace

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.

Oct 22 '18 17:10 dead-claudia

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.

Oct 22 '18 18:10 dead-claudia

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

removing document.activeElement seems to work