[wp-trac] [WordPress Trac] #11172: Login form template function

WordPress Trac wp-trac at lists.automattic.com
Tue Dec 22 21:50:49 UTC 2009


#11172: Login form template function
----------------------------+-----------------------------------------------
 Reporter:  ryan            |       Owner:  beaulebens
     Type:  task (blessed)  |      Status:  assigned  
 Priority:  normal          |   Milestone:  3.0       
Component:  Template        |     Version:  2.8.5     
 Severity:  normal          |    Keywords:  has-patch 
----------------------------+-----------------------------------------------

Comment(by filosofo):

 I think having a pluggable login form is a good idea, but I think we need
 to put some more work into this implementation.

  * Usually if a WP function returns a string rather than printing it, it's
 named something like {{{/function (wp_)?get.*/}}}
  * We should decouple the test for whether someone is logged in from the
 logic that generates the form markup.  It would be maddening to call a
 function that's supposed to generate form markup and have it return an
 empty string, because it decides based on global variables that you don't
 actually need the form markup you requested.
  * All of those id attributes should be at least filterable.  Even better,
 just give the form itself an id and remove all the others (let classes
 handle styling).
  * Use a filter instead of a ternary operator for the "remember me" bit.
  * Better yet, just let all of that stuff---ids, whether to show the
 remember me checkbox, etc., be set by an array of default arguments.  Then
 you can have just one, final filter for everything.
  * Use more descriptively precise filter names.  For example,
 'login_form_username' is not very helpful here, because it filters the
 ''label'', not the username itself.
  * Have more semantically-rich class names for the form elements'
 wrappers. E.g. {{{ <div class="form-element username-wrapper">}}} or
 something like that.  Then you're freed to do more with the styling.

  * Shouldn't the username value and remember-me checkbox have the
 possibility of being set?
  * This looks like it's ripe for a security problem:
 {{{
 $redirect = ( is_ssl() ? 'https://' : 'http://' ) . $_SERVER['HTTP_HOST']
 . $_SERVER['REQUEST_URI'];
 }}}

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/11172#comment:8>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list