codemirror / CodeMirror-v1

An editable-iframe based code editor in JavaScript. See https://github.com/marijnh/CodeMirror for the currently maintained version
http://codemirror.net/
Other
362 stars 63 forks source link

HTML5 required attribute breaks hack for form submission #59

Closed qpleple closed 13 years ago

qpleple commented 13 years ago

Using the CodeMirror.fromTextArea() method, the textarea is hidden and replaced by the CodeMirror element.

Submission is done by the hack:

if (textarea.form) {
  // Deplorable hack to make the submit method do the right thing.
  var rmSubmit = connect(textarea.form, "submit", save, true);
  if (typeof textarea.form.submit == "function") {
    var realSubmit = textarea.form.submit;
    function wrappedSubmit() {
      save();
      textarea.form.submit = realSubmit;
      textarea.form.submit();
      textarea.form.submit = wrappedSubmit;
    }
    textarea.form.submit = wrappedSubmit;
  }
}

If the real (and hidden) textarea has the HTML5 required attribute, modern browsers check the textarea value before calling textarea.fom.submit() (so before "saving" the data) and tries to display an error notice.

The error notice may fail because the textarea is not focusable (it does on my browser, Chrome 15.0).

In short, the form is never submitted and no error is displayed.

marijnh commented 13 years ago

The problem seems to be that A) the validation attributes are checked before the onsubmit event is fired, so if CodeMirror clears the textarea, it'll be happily submitted, because it had something in it before, and B) the hint that pops up when the form is not submitted (which happens when the texarea was empty to begin with), seems to be placed in such an incoherent way (at least on Chrome) that it either doesn't show up at all (when the textarea has an absolute position) or ends up way in the wrong place (when the textarea is given a height of 0).

There doesn't appear to be a way to make CodeMirror play nice with such attributes, short of reimplementing them, which is out of CodeMirror's scope.

qpleple commented 13 years ago

Yes indeed.

So what do you choose? Saying that CodeMirror work only without requiredattribute or maybe find a fix?

If you don't want to extend CodeMirror scope to reimplementing such attributes, would you accept checks for these attributes and notice if they're here ?

marijnh commented 13 years ago

It could easily notice the attribute, or some other HTML5 validation attribute, is there, but how would it respond?

qpleple commented 13 years ago

Not creating the CodeMirror object and fail with a log message so that the developer knows something is wrong and that he has to remove the requiredattribute to make it work.

marijnh commented 13 years ago

I'd say it is the library user's responsibility to not create CodeMirror instances for his/her textareas in such a case. There are a lot of other broken ways to use CodeMirror, and I don't want to start testing for all of them in the library (mostly for file size reasons -- such extra code adds up).

qpleple commented 13 years ago

All right, fair enough.