Make WordPress Core

Opened 5 years ago

Last modified 18 months ago

#15384 new enhancement

wp-login.php refactor

Reported by: nacin Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: 3.2-early
Focuses: Cc:


wp-login.php needs some serious work. When looking to do some improvements in #5919, I realized I literally needed a goto in order to achieve the goals outlined in this comment:


I am thinking a WP_Login class with some methods that can handle various different forms, POST handling, and rerouting.

Attachments (6)

15384-2010-12-17.diff (59.3 KB) - added by norbertm 5 years ago.
15384-2010-12-20.diff (61.7 KB) - added by norbertm 5 years ago.
wp-login.php (1006 bytes) - added by norbertm 5 years ago.
class-wp-login.php (32.7 KB) - added by norbertm 5 years ago.
class-wp-login-20101221.php (35.9 KB) - added by norbertm 5 years ago.
php5 oop version
class-wp-login-20101222-r17107.php (36.1 KB) - added by norbertm 5 years ago.
included comment:10 and merged everything from trunk up till r17107

Download all attachments as: .zip

Change History (32)

comment:1 @holizz5 years ago

  • Cc tom@… added

comment:3 @norbertm5 years ago

  • Cc norbert@… added

@norbertm5 years ago

comment:4 @norbertm5 years ago

Included an early version of the refactored WP_Login class.

The generic idea is to encapsulate all initialization, a dispatch controller, several action handlers (handle_*, process_*), some service layer methods (do_*) and separate view rendering methods (render_*) from the original wp-login.php script.

Ideally, I'd like to separate requests that do not change state (GET) from those that do (POST) but the current action handlers do not let that happen easily. The idea is if it's a POST request handle that part first, collect any errors, then proceed with the GET part to compose a response by passing the errors to that method.

handle_login() needs some rethinking, especially at the end.

I'd like improve smaller things, for example, the success branch should return at the end of a function vs exit in the middle buried within if()s.

Interested to see if redirects are worth extracting out into their own methods.

In the end, I plan to update the entire file to follow the WP coding standards.

Extracted $aliases to serve as a map for duplicate (deprecated?) action names.

Any feedback is greatly appreciated.

@norbertm5 years ago

comment:5 @norbertm5 years ago

Another update.

The class is composed like this:

  • "Dispatch" routes the request to the proper action.
  • "Process" functions do the action processing and return errors if any. They call "Do" functions to do the actual service level work.
  • Finally the errors returned are passed to a "Render" function, which in turn calls one or many "Output" functions to output the HTML.

The class is organized so that PHP code is at the top and HTML at the bottom unless there's a better place for HTML.

It's still a work in progress but I'd love to get some feedback. What do you think so far?

@norbertm5 years ago

@norbertm5 years ago

comment:6 @norbertm5 years ago

Taken a more object-oriented approach that is still PHP 4 compatible.

Even though I don't want to go too far with OOP, I really want to extract all HTML rendering functions into their own view classes because it's still a terrible mess.

Attached full files instead of a diff because it's a large set of changes.

comment:7 @nacin5 years ago

No need to be PHP 4 compat in 3.2.

comment:8 @norbertm5 years ago


Indeed. That's awesome! Let me switch gears then...

@norbertm5 years ago

php5 oop version

comment:9 @norbertm5 years ago

Created a PHP 5 OOP version that provides clarity and straightforwardness:

  • separates controllers and views
  • separates PHP and HTML code
  • one step closer to testability.

Simplified and reduced the kinds of methods compared to the previous version.

If you think it's too much OOP we can iterate but I would really prefer going forward internally while still providing hooks for the end users.


comment:10 @norbertm5 years ago

Re: ticket description, it would be easy now to display the login form with a message after password reset simply by making the class at the bottom extend WP_Login_View_Login like this:

 * Outputs the login form with the password successfully reset message.
class WP_Login_View_ResetPass_Completed extends WP_Login_View_Login {
	 * Class constructor.
	public function __construct() {
		parent::__construct( '<p class="message reset-pass">' . __('Your password has been reset.') . '</p>' );

That's it.

Alternatively, you could create a new view and return it from any controller at any point to implement custom functionality.

comment:11 @norbertm5 years ago

The same can be accomplished with a single WP_Login class and many functions of course. It would still make sense to keep a separate dispatch and many process_* and render_* functions to separate controller and view logic.

Any of this works for me, let me know.

@norbertm5 years ago

included comment:10 and merged everything from trunk up till r17107

comment:12 @neoxx5 years ago

  • Cc neo@… added

Nice to see a rework of good old wp-login.php. What about a few filters for the output as I've mentioned in #10006 ?

Another thought: Shouldn't we stop sending passwords in plain-text emails? (as we now also don't do for lost password requests #5919?) I mentioned that already in comment:39:ticket:2870.

comment:13 @Viper007Bond4 years ago

It'd be cool if you could pre-fill the lost password form using a query argument. It's a simple change -- just change $_POST to $_REQUEST:

$user_login = isset($_POST['user_login']) ? stripslashes($_POST['user_login']) : '';
$user_login = isset($_REQUEST['user_login']) ? stripslashes($_REQUEST['user_login']) : '';

comment:14 @sboisvert4 years ago

  • Cc stboisvert@… added

comment:16 @johnbillion4 years ago

  • Cc johnbillion added

comment:17 @jeffstieler4 years ago

  • Cc jeff@… added

comment:18 @beaulebens3 years ago

  • Cc beau@… added

comment:20 @scribu3 years ago

  • Type changed from defect (bug) to enhancement

I think having two classes (a controller and a renderer) for each view is a bit much.

We should have a single set of functions for outputting the various HTML bits, which are called by each controller with the appropriate parameters.

comment:21 @scottconnerly3 years ago

  • Cc scott@… added

comment:22 @zombrows3 years ago

  • Cc zombrows added

comment:23 @iseulde2 years ago

  • Cc avrylnoxke@… added

comment:24 @iandunn22 months ago

  • Cc ian.dunn@… added

comment:25 @jeremyfelt20 months ago

  • Component changed from Administration to Login and Registration

comment:26 @ircbot18 months ago

This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.

Note: See TracTickets for help on using tickets.