r75670 MediaWiki - Code Review archive

+wfValidateEmail = function( mailtxt ) {

Don't use the wf prefix for function names in JavaScript. Contrary to what you may think, it's also not a global function unless you happen to be in debug mode.

+ * according to HTML5 specification. Please note the specification

+ * does not validate a domain with one character.

You mean it doesn't validate a TLD. Each part of the domain name can have only one character if you like.

+		// "apostrophe"
+		'@'

This is an at-sign, not an apostrophe.

Also, this is wrong, you're misreading the spec:

+        // Domain first character
+        '[' + rfc1034_ldh_str + ']'
+        +
+		// second char and following can include dot
+		'[' + rfc1034_ldh_str + '\\.' + ']' + '+'

It should be

		// Domain first part
		'[' + rfc1034_ldh_str + ']+'
		+
		// second char and following can include dot
		'([.' + rfc1034_ldh_str + '])+'

Your version allows crazy domain names like "f.......", which is not what the spec says. It allows two or more dot-separated parts.

Acknowledged. I have missread the ABNF definition for ldh-str :-/ I will patch both the php and js. Thanks!

Okay, the UI here could use a lot of improvement. Take a look at this study: http://www.alistapart.com/articles/inline-validation-in-web-forms/ Two major conclusions of relevance:

1. Do not display success/failure onkeyup. Instead, display it onchange. That way the user isn't told their e-mail address is invalid just because they've only half-entered it ("well, of course it's not valid yet!"). This means they get no client-side message if they just hit enter right away, but that's okay, because
2. Don't display success, only failure. People get confused when they see their answer to an obvious question marked "correct". From the study:

   Our participants knew we had no way to know their correct name or postal code, so they knew that the green check mark didn’t mean “correct.” But what did they think it meant? They weren’t sure—and that was the problem. Not knowing what the message meant, our participants stopped to ask questions of the moderator instead of confidently answering what were very easy questions.

Also, you seem to have hardcoded the message as English . . . ?

1) onChange is not useful when this part of the form only has one input. It is more about changing the email then either hitting enter or clicking submit, although one could just disable the submit button until the error is address. I found the onKey event makes it much more dynamic and fun to use.

2) An email might be obvious for us, not for everyone. I got friends and family members who do not even know how to do the @ sign ! They have to copy-paste it from another source. As for the correct postal code, I totally agree with the study, probably why I said "looks valid".

I have hardcoded the message since I am not sure yet how to handle i18n for javascript messages. It looks like a work in progress.

1) Inline validation doesn't matter so much if the user submits right away. After all, then they get feedback in just a second regardless. It's most useful if the user is filling out several things at once. But if you think otherwise, add an onsubmit trigger too.

Your assessment of what's fun to use doesn't outweigh an actual usability study. The study found that users clearly preferred being told that their input is wrong only after they've finished filling in the particular input. You're going to have to bring actual evidence if you feel otherwise.

2) It's arguable whether it's comparable to the postal code example, particularly since you say "Looks valid". But I still don't think the notification of validity serves any purpose here. At best it's distracting.

1) Since the user will submits right away, I found the onKey event more useful. That save it a click. I am not willing to add the onSubmit trigger since we could get a bug and I would prefer having some feedback before.

I have read the study (interesting stuff indeed, thanks for it) which apply to a multiple input form. The video show how annoying and distracting it is to have instant validation when filling a multiple input form. In our case, I have the feeling that instant validation is not as distractive since there is only one input. As for the usability, I only have my girlfriend (and you) to test upon :-p The first preferred the changing background ;-b

2) The whole idea was to warn users about obvious errors. For example, someone copy pasting : "Ashar Voultoiz" <hashar at free dot fr>, or unintended text which might precede or follow an email when copy pasting.

I suggest we ask the usability people for an opinion, since we have a whole bunch lying around.

Validation needs to happen on the server, and optionally the client. The change event is very odd and in some browsers buggy, usually not working the way you expect. The correct way to perform this validation is on blur. Submitting the form would cause the submit button to be focused, blurring the text field, and thus validating the field. The submit handler should not allow the form to be submitted if the field is empty or marked as invalid.

I have added the server side check (includes/Preference.php) with r75682 and r75876 (the later fixes the regexp).

I do not like the blur event since it triggers on a focus change only. Thus, this event defeats the feature I want to implement : provide an "instant" tip about email state. This is much like the skins/common/mwsuggest.js script or the twitter signup page.

I know the keyup / keydown / keypress are not consistent between browsers, specially when you actually care about the key pressed. In this case, we just need to know that a key got pressed which usually happen when typing it or copy pasting (since it press the V or whatever key).

(Trevor, if you have any documentation from the usability project, I am HIGHLY interested!)

"Submitting the form would cause the submit button to be focused, blurring the text field, and thus validating the field."

Is that true? This page seems to work otherwise:

data:text/html,<!doctype html>
<form>
<input name=foo onblur="alert(true)">
<input type=submit>
</form>

If you focus the input and hit enter, it submits but doesn't alert, at least in Firefox 4.

Perhaps you would need to put a $( 'submit button id' ).focus() in the submit handler...

It's okay to do client side validation as *suggestive* but never *canonical*; validation has to happen on the server.

I'd not use onKeyUp or onChange; do your stuff on onBlur.

I agree, when using unkeyup it's impossible to do a valid form submission without seeing errors (since it would be invalid while typing), which is bad because it should always be possible to fill in a form and submit it without seeing any errors.

Should I just remove the whole thing ? The main reasons are :

It is not i18n compliant. One way would have been to add hidden messages in the HTML and display/hide them on purpose. The problem is that it needs some new exceptions in HTMLForm and non CSS browser users will complain.

Originally, I wanted to warn the user ASAP (by using onKeyUp) but it seems to be a bad practice. The consensus seems to be using onBlur and a validation function on the submit buttons. This was not really what I wanted to do.

This seems to be controversial, so let's back it out of 1.17 at least.

Marking new: r79904 adds i18n support (thanks Roan) r79910 use onBlur()

This can be marked resolved now.