1. I fix the bug with the 'page arguments', it needs to be an array().
2. I got rid of the loop in hook_form_alter(), in the original code, the loop continue after js is added. This is unnecessary and inefficient.
3. I use a different jQuery selector expression ':input:visible:enabled:first'. It should cover both textfield and textarea and exclude hidden fields. In fact, it cover any kind of input control.
4. I enable the user_login_block form for focus. This is the form user module put in the login block. This one is odd, the id is 'user_login_block' but in HTML, it's 'user-login-form'. I added some text in the field description to make clear this oddity.
I made these changes in the D6 HEAD version.
| Comment | File | Size | Author |
|---|---|---|---|
| d6_patch.diff | 2.91 KB | mattyoung |
Comments
Comment #1
anders.fajerson commented1. Nice.
2. Oops, that's some bad code. Great fix!
3. Interesting, and it covers textareas too, have to test.
4. Not sure about this one. The drawback with focusing fields is that backspace (to go back) or other keyboard commands won't work. So you should really only focus when you're sure that the users intend is to use the form. It's not the case with the block which can turn up on every page for anonymous users, and they might not intend to log in. Makes sense?
I also notice that your patch didn't use the standars format. Could be good to use if you want to contribute more patches (you should!). Here's some documentation: http://drupal.org/patch/create
Thanks for the great work.
Comment #2
mattyoung commented>4. Not sure about this one. The drawback with focusing fields is that backspace (to go back) or other keyboard commands won't work. So you should really only focus when you're sure that the users intend is to use the form. It's not the case with the block which can turn up on every page for anonymous users, and they might not intend to log in. Makes sense?
Agree. Perhaps leave the note in description stating this form id is different and let the user decide whether to enable.
Thanks for the pointer about patch. I created the path using
cvs diffinstead ofcvs diff -up.Comment #3
anders.fajerson commentedFixed: http://drupal.org/cvs?commit=326010