As stated in the summary, currently if one enters an incorrect value for the input element, the submit is blocked per HTML5 Forms, but the entry is recorded by the Form Manager. This shouldn't happen.

Hmm, tough call on blocking. I'm going to minus this, but it's definitely wanted. Form Manager already has a bug (filed, I think) where if a page cancels the form submission, we will already have saved the form's data. So I think this is an existing problem, though HTML5 form validation will certainly make it more visible as it enters use. I suspect the fix here shouldn't be too complex -- probably need to add a new |afterformsubmit| event, and save the form data there. There's a chance this will cause some odd results -- the |earlyformsubmit| observer (which gets called before the normal |formsubmit| listeners) was added a long time ago because some sites were modifying form data during the submission, so the saved data != the entered data. I don't know if this is still an real problem on the web... If it is, we could probably make the |earlyformsubmit| observer just queue up the data -- as formSubmitListener.js already does due to E10S -- and only commit it to the DB upon |afterformsubmit|.

I wrote a patch moving the CheckValidity() method _before_ the form submission stuff. As a side effect, this bug should be fixed. Can you confirm that?

When we try to submit an invalid form, nsHTMLFormElement::OnSubmitClickBegin is called which is calling nsIFormSubmitObserver::notify, making the form history to save the values.

Since we reached interface freeze, I'm wondering how far we can go here. There would be a simple and dirty fix which would consist of checking that mInvalidElementsCount is null before calling NotifySubmitObservers in nsHTMLFormElement::OnSubmitClickBegin. Other solutions would probably require interface changes or adding a new interface but calling methods from the new and the old one. If we can't change the interfaces, I would prefer the dirty hack instead of adding a new interface and changing the interfaces after branching. Dolske, what would you recommend?

This is a fix that should be removed with bug 610402 when we will be authorized tho change interfaces.

Comment on attachment 488895 [details] [diff] [review] Patch v1 I actually need a review from a content peer here. Dolske, I still want to know if the way I'm solving this seems fine or if you would prefer the other way (which doesn't seem doable with the interface freeze).

Comment on attachment 488895 [details] [diff] [review] Patch v1 Please do not request approval until all reviews/feedback are complete.

Pushed: http://hg.mozilla.org/mozilla-central/rev/60f75da9765b