jquery-ujs
jquery-ujs copied to clipboard
rails
Unexpected default behavior for browsers that don't support required input fields / form validation
rails.js adds a behavior to forms so that these don't get submitted if there is a blank required input field. This results in unexpected behavior for browsers that don't support form validation.
The following demo uses jQuery 1.9.1 and the current rails.js. Open http://jsfiddle.net/wqWMf/4/ in the current version of Safari (6.0.3) and hit the submit button.
Current behavior: Nothing happens
Expected behavior: Either a) the form should get submitted although the required field is missing or b) jquery-ujs should add some default styling to highlight the missing fields.
Everything works as expected with browsers that support form validation.
I think the behavior should either be removed or we should have to opt-in explicitly like we are doing with the data-remote, data-confirm,… attributes.
Living in Railsland we know hot show error-messages coming from the server-side validations in forms. The current behavior breaks this, because the form does not get submitted and no error-messages get rendered.
It's meant to act as a polyfill for browsers that don't prevent the form from being submitted. Since some do, it's always a good idea to handle it yourself. What we don't want is someone assuming that the form can be submitted with blank required fields just because the browser they're using to develop and test happens to allow it (thus not realizing that their site would be broken in other browsers). This way, they'll see it no matter what, whether their browser of choice supports it or not.
What we don't want is someone assuming that the form can be submitted with blank required fields[…].
Why would any developer add required attributes to input fields and still assume that the form can be submitted with blank fields? If i mark a field as required, i would never assume that it really isn't – unless the browser does not support form validation. If this is meant as a polyfill, i think it's not a good one, because it breaks the UX on some browsers (see above).
The demo shows a bug, caused by rails.js and me trusting my assumptions how required fields would work (or not work). You are right that it's always a good idea to handle form validation yourself, but making the form act in unexpected ways does not help. This behavior also works around showing custom errormessages coming from the Rails app, SimpleForm, Formtastic or the like. It's still useful to add these error messages on the server-side, so why break this?
Basically i just can't think of a use case for this to be the default behavior.
Why would any developer add required attributes to input fields and still assume that the form can be submitted with blank fields?
Because some browsers don't prevent the forms from being submitted. In fact, from your original post:
Expected behavior: Either a) the form should get submitted although the required field is missing
It's not that we're making the form act in unexpected ways, it's that we're making it act the same way in all browsers.
Basically i just can't think of a use case for this to be the default behavior.
The thing is, it is the default behavior in some browsers, we didn't make it that way.
We could possible add some sort of default styling to complete the polyfill, but that wasn't part of the required field spec, so it wasn't implemented initially.
We could possible add some sort of default styling to complete the polyfill
:+1: to that. we've had instances of developers going down rabbit holes trying to figure out why a form won't submit before putting the clues together. something obvious would be a big help.
You are not making it act the same way in all browsers, because it lacks the visual feedback, which is essential. I would rather like that behavior to be removed, instead of completing the polyfill. There are enough polyfills already out there. Looking forward to hearing some more opinions about this.
I'm definitely all ears if anyone else has opinions on this. I'd prefer not to remove it, because like I said, browsers then act really differently, and it'd be easy to push out an app that's broken in some browsers due to the difference in behavior, and never know, which violates the element of least surprise.
I ran into this issue as well, and I completely agree with @ahx. If this is meant to be a polyfill, it needs to include the visual feedback. As it currently stands, forms are broken in browsers that don't support validation natively.
Please remove this behaviour. I wasted so much time trying to debug what was causing the form submission to fail.
- The intention is to make all browsers act the same. But this goes only half the way and hence make it very confusing. In browsers which support required attributes, there is a visual indicator on the field giving immediate feedback. jquery-ujs just aborts the submit event with no visual feedback (or even a message to console). So if someone wants to make use of this, they will have implement the UI part. At that point it is better to use something like jquery-validate which is well tested.
- Also why is this the default? jquery-ujs should not be modifying an ordinary form. If this is retained, this should be opt-in
- "push out an app that's broken in some browsers" - How? Do you mean to say, people will not include error messages because the browser they are using will not allow form submission with blank required fields?
@JangoSteve super to hear you're all ears! As the other commenters have already suggested, the lack of any visual feedback brakes the principle of least surprise (for us at least). My vote would be to implement some sort of visual feedback, even if it were an alert.
Is there a quick way for developers to disable this in the meantime?
@michrome , I'm late to the party, but came across this issue as I was researching an article I wrote on UJS. If the form is marked as novalidate then both this polyfill and native browser validation will not run. If you want to disable only the polyfill, you should be able to override $.rails.requiredInputSelector to be a selector that will be empty. I think this is the easiest, least impactful way to do this but I haven't actually tested it.
For anyone else running into this, you can get the form to respond like so:
$('#my_form').on('ajax:aborted:required', handleErrors)
I stumbled on this thread from @thoughtbot's article on 'jQuery UJS'. I must confess it's been months ... this has been baffling me. Forms on Safari wouldn't not submit ... on Chrome there's visual feedback on required fields.
I share @j-manu's sentiments.
We just ran into this issue as well.
Firstly it baffled us that Safari 8 does not implement HTML5 validations, but when we created a jsFiddle we were noticing different behaviour. When we started to track down the dozens of javascript widgets on our site, we narrowed it down to jquery_ujs.
My initial thought was "Why on earth is jquery.rails trying to implement browser behaviour?". If I wanted a polyfill, I would have added one. My preference is to at least have the browser default behaviour occur. On Safari, that would mean a form is submitted, the validation errors are returned by rails in the response and the user can action them.
In the current implementation, the user clicks 'Submit' and nothing happens. Not even a log to the console telling the developer why the submit was cancelled.
I think this is clearly a case of a library going beyond its expected behaviour.
I agree that this behaviour shouldn't be part of jquery-ujs.
This little library is supposed to be "unobtrusive" (from the description itself). But it is extremely obtrusive changing the default behaviour.
If this is a shim/polyfill then it should be explicitly stated that way.
If it IS a polyfill, then why doesn't it implement all other HTML5 built in validations?
There are plenty of validation libraries and it's not the responsibility of "unbotrisuve" UJS to deal with form validations.
In addition to my previous comment, it's confusing that the ajax:aborted:required event is fired, even on a standard, non-remote form. There's no AJAX taking place here. If it was a remote form, then maybe I could understand the behaviour - but not for standard forms.
Being pragmatic I'd not remove the feature right now. Removing it will require a major bump since may have some people expecting jquery-ujs to do the polyfill.
I think we can do:
- Implement the visual feedback
- Make easier and document a way to disable the polyfill.
- Disable the polyfill by default in new applications
- Flip the default sometime in the future.
cc @lucasmazza
All of that makes sense to me except for the first one. I'm not sure it's possible to do that in a way that is going to make anyone happy.
On Wednesday, January 14, 2015, Rafael Mendonça França < [email protected]> wrote:
Being pragmatic I'd not remove the feature right now. Removing it will require a major bump since may have some people expecting jquery-ujs to do the polyfill.
I think we can do:
- Implement the visual feedback
- Make easier and document a way to disable the polyfill.
- Disable the polyfill by default in new applications
- Flip the default sometime in the future.
cc @lucasmazza https://github.com/lucasmazza
— Reply to this email directly or view it on GitHub https://github.com/rails/jquery-ujs/issues/319#issuecomment-69916018.
FYI, the reason we didn't originally try to add default styles based on the event, was that it'd make it much harder then to add your own styles, as you'd have to overwrite the default styles before you could customize the app with your own. Also it was just more work, so it can more be though of as something no one has yet gotten around to adding, and we've never received a pull request for it either.
That said, here are some better answers to some of the other comments/questions in the thread.
@asafbrukarz
In addition to my previous comment, it's confusing that the ajax:aborted:required event is fired, even on a standard, non-remote form.
This library is not an AJAX-only library, it does a lot of other things that aren't related to AJAX functionality. If it's confusing that this library is doing it for non-remote forms, that might be due to a misunderstanding of what this library does (which could probably be fixed with better documentation).
Here's a quick list of what it does:
- remote links and forms
- method links (that use e.g. method: :put, :post, : delete)
- confirm links (that use e.g. confirm: true)
- disable elements and disable-with elements
- normal forms that incorrect the authenticity token directly and are cashed in your HTML (normal forms are fine, bit if you cache them, UJS helps replace the stake authenticity tokens)
- jQuery ajax methods called directly outside of remote links and forms (UJS automatically spend authenticity tokens to all jQuery ajax requests you create so that your controller doesn't reject them and log you out)
@j-manu
The intention is to make all browsers act the same. But this goes only half the way and hence make it very confusing.
I agree that it goes half-way. Ideally, some default styling or other indicator (maybe even as simple as a console.log if the console is open) would help.
Also why is this the default? jquery-ujs should not be modifying an ordinary form. If this is retained, this should be opt-in
As stated earlier, jquery-ujs is not an AJAX-only library.
"push out an app that's broken in some browsers" - How? Do you mean to say, people will not include error messages because the browser they are using will not allow form submission with blank required fields?
I mean that, because not all browsers behave to spec regarding required fields, developers may be developing and testing on a browser that treats required fields one way, then deploy, not realizing that some users are using a browser that treats required fields differently.
You could argue that if a developer is using the required HTML5 attribute, then it's up to them to know browser compatibility, which I would agree with. Though, in this case I think the thinking was that because they're using a rails helper (i.e. :required => true), they may not know that the rails helper attribute is being converted into an HTML5 attribute that isn't fully cross-browser supported.
That being said, support seems much better now than it was when this was implemented.
@rafaelfranca Given the better browser support today, and the fact that most developers probably won't be relying on their front-end to do their form validation (so much that they're not re-validating on the back end), it's probably better now than it would have been before to let the very few browsers that don't support the required attribute erroneously submit blank data than to try to stop them and potentially confusing people. So moving toward removal may not be a bad idea.
I agree that it'll warrant a major version bump when we do it though. I did a quick code search on Github, and it seems like there are a lot of libraries that bind to this event and use it.
@JangoSteve you are missing the point people are making here.
The questions for you to answer if you can.
The jquery_ujs reimplements the logic of required thus acts as polyfil. Correct?
Why is it never mentioned in the docs anywhere because it is totally undesired behaviour from this library (as we all can see on this issue)?
If this library IS a polyfil, then why only the required is implemented and not all of the validations?
It just doesn't make sense, breaks the expected behaviour, makes things a lot more complicated and hard to track down.
Also if you'll look closer at your own quick code search on Guthub you'll see that lots of usages of that event are "workarounds" that use the wrong library for the job and aren't reliable anyway.
@dnagir I think you missed all of the points that I made. No one here is disagreeing with you. From my previous comments, my recommendation is to either add the default styling, or even to remove it now. It seemed like a good idea at the time (who knows, maybe it was). At any rate, I think it's less needed now as most browsers have caught up anyway, and we can just let the browsers all handle it.
The only point I was making with the code search on github is that we can't outright remove it without doing a major version bump (which shouldn't be an issue for anyone here), because of all the code that is already relying it (whether you consider it to be desired behavior or not, removing it will break code).
To answer your questions specifically though...
The jquery_ujs reimplements the logic of required thus acts as polyfil. Correct?
No, it doesn't reimplement the required logic per-se, only the part that seemed broken on a few specific browsers. Even then it wasn't seeking to make required fully functional on all browsers, only to make the browsers at least act the same (though on that point, we all know it fell short in not providing some default styling). So, no, it wasn't trying to act fully as a polyfill, just trying to get some consistency across browsers (which is about half the job of a polyfill, the other half being to implement missing functionality which this wasn't trying to do).
I don't understand what difference it makes though, as to whether this particular piece of functionality in jquery-ujs is labeled a polyfill or not. How we decide to move forward should be based on reason and merit, not labels.
Why is it never mentioned in the docs anywhere because it is totally undesired behaviour from this library (as we all can see on this issue)?
Because the docs are incomplete apparently. The good news is, the jquery-ujs docs are a wiki. It should be added. Though again, I'm of the mind that we should move toward removing this functionality now anyway.
Also, just FYI, we can't take the comments in one issue as evidence that something is "totally undesired behavior". What's undesired to one person may be desired to another. Anyone for whom this actually was desired behavior wouldn't necessarily speak up here or in any other issue, because it functions as desired for them, and so they wouldn't be reading through this thread or create a ticket for something that works for them.
To say it's undesired for you and explain why helps us, but claiming that it is factually undesired "as we can all see on this issue" isn't valid in the bigger picture of maintaining a project used by many people, not just the people commenting on a particular issue thread.
That said, again though, based on of yours and others discussed in this thread, I agree and can see how it'd be undesired.
If this library IS a polyfil, then why only the required is implemented and not all of the validations?
It's not. But the reason for why required and not other validations is because the required field had more cross-browser compatibility issues than the form validations based on input type.
It just doesn't make sense, breaks the expected behaviour, makes things a lot more complicated and hard to track down.
The original thinking was that it'd be better to have to track down an issue that you know you have because you can see it happening, than to push out code that appears to work but silently fails on other browsers that you didn't think to test.
Again though, I think it should probably be removed.
@asafbrukarz In answering your question earlier, I totally missed the detail that your point was due to the fact the event is called ajax:aborted:required rather than to a misunderstanding as to the responsibilities of the library.
Yeah, that's a bad name for the event. Originally (when it was added and named), it was only for remote forms. Adding it for non-remote forms came later, and we just didn't change the name. I'd say we should change it, if I didn't think we should probably just move toward removing it instead.
@JangoSteve I appreciate the comments you've made to this issue. Naturally, this functionality has been in place for several versions (back to April 2013 as far as I can see) and you can't simply remove it as some users have built their apps to work around it.
For our app, we had to make the decision to either fork and patch jquery_ujs, or work around it by implementing the ajax:aborted:required event. We chose the latter, as it was the path of least resistance.
When we decided to implement HTML5 attributes on forms (using SimpleForm - thanks @rafaelfranca), we read through caniuse.com to determine the current implementation status across a variety of browsers. We decided that for the browsers that support it, we could leave it enabled, and for those that don't, our server-side validations take care of it. Per best practise, we never trust that the client's browser has correctly validated anything, and always assume that we have to validate user input - regardless of whether it is an XHR, API or HTTP request.
The bug that we discovered in our app (Safari doesn't submit forms with an empty required input), was difficult to track down, because it wasn't linked to any Javascript that we had written.
Ultimately, I don't mind whether the feature is in the library or not (although my personal belief is that it doesn't belong). My preference is to make it opt-in, like many of the other features. The fact that even input written in plain HTML, not just the ones generated with f.input required: true are affected makes the implementation of this feature an issue.
Also, in relation to the proposal to add default styling, my request would be to make this purely opt-in. I'm not aware of any other instance where the library adds default styling, and we already have a tough enough time trying to reset browser styling.
We use Bootstrap so our implementation looks like this:
$("form").on 'ajax:aborted:required', (e, data) ->
data.each ->
$(this).parents('.form-group').addClass('has-error')
Styling is added to a parent div, not to the element itself. While there may be libraries using this functionality at the moment, adding styling where the was none before will cause anyone using this functionality to wonder why some forms look different all of a sudden.
What's the status on removing the required input validation? Would a PR be welcome at this point?
Just spent ages tracking down a bug with form submission due to this polyfill, and ended up removing jquery-ujs from our app as a result. +1 to making it optional!
Add another to the pile of developers caught by this. Sunk four hours into trying to work out why my form was not submitting as expected to hit the server-side validations in iOS before finding this. Only really tweaked when the form would also not submit in IE9, then got lucky with a Google search to get here. Never occurred to me that the ujs library added by default in Rails was causing this behaviour.
For me it's not about adding more custom JavaScript for the non-supporting browsers, it's trusting that the behaviour I expect from the older browser (just submit the damned form and let the server sort it out) will work. Not submitting by default AND having no UI feedback by default is not okay.
For the end-user the behaviour was awful. Looked like the button was broken until all the form fields were perfectly filled in.
Please reconsider, the current behaviour feels like a nasty gotcha, especially as it's the default behaviour in Rails.
Update: I'm not sure how evil this is, but it's working for me, for now at least. After including the jquery-ujs library, you can monkey patch the blankInputs function to return false which disables this behaviour. I've added a little extra to not disturb the nonBlankInputs function which calls the original.
$.rails.blankInputsOriginal = $.rails.blankInputs;
$.rails.blankInputs = function() { return false; };
$.rails.nonBlankInputs = function(form, specifiedSelector) {
return rails.blankInputsOriginal(form, specifiedSelector, true); // true specifies nonBlank
};
... saves having to bind the ajax:aborted:required callback on every form on the site.
Sorry, but this is not expected behaviour to anyone but the person who wrote it. For end users, it's juts a broken form (you try to submit it and absolutely nothing happens). For developers, it's just an hard to trace bug -- as we've seen from previous examples -- because no developer would imagine jquery-ujs would do such thing.
If a browser does not support client side validation the expected behaviour is that the form be submitted without validation. As it stands, you are forcing developers to implement form validation shims/polyfills for Safari or to hack jquery-ujs.
For what it's worth, a change was just merged to rails-ujs (the jquery-ujs replacement coming in Rails 5.1), that removes this polyfill.
I'd expect jquery-ujs will continue to keep this polyfill. The simplest way to disable it I've found is to add the following after requiring jquery-ujs:
$.rails.requiredInputSelector = 'empty-selector';