WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 7 weeks 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:

Description

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:

http://core.trac.wordpress.org/ticket/5919#comment:39

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 3 years ago.
15384-2010-12-20.diff (61.7 KB) - added by norbertm 3 years ago.
wp-login.php (1006 bytes) - added by norbertm 3 years ago.
class-wp-login.php (32.7 KB) - added by norbertm 3 years ago.
class-wp-login-20101221.php (35.9 KB) - added by norbertm 3 years ago.
php5 oop version
class-wp-login-20101222-r17107.php (36.1 KB) - added by norbertm 3 years ago.
included comment:10 and merged everything from trunk up till r17107

Download all attachments as: .zip

Change History (32)

comment:1 holizz3 years ago

  • Cc tom@… added

comment:3 norbertm3 years ago

  • Cc norbert@… added

norbertm3 years ago

comment:4 norbertm3 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.

norbertm3 years ago

comment:5 norbertm3 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?

norbertm3 years ago

norbertm3 years ago

comment:6 norbertm3 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 nacin3 years ago

No need to be PHP 4 compat in 3.2.

comment:8 norbertm3 years ago

http://codex.wordpress.org/Switching_to_PHP5

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

norbertm3 years ago

php5 oop version

comment:9 norbertm3 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.

Thoughts?

comment:10 norbertm3 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 norbertm3 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.

norbertm3 years ago

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

comment:12 neoxx3 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 Viper007Bond3 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 sboisvert3 years ago

  • Cc stboisvert@… added

comment:16 johnbillion2 years ago

  • Cc johnbillion added

comment:17 jeffstieler2 years ago

  • Cc jeff@… added

comment:18 beaulebens2 years ago

  • Cc beau@… added

comment:20 scribu2 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 scottconnerly23 months ago

  • Cc scott@… added

comment:22 zombrows23 months ago

  • Cc zombrows added

comment:23 avryl9 months ago

  • Cc avrylnoxke@… added

comment:24 iandunn6 months ago

  • Cc ian.dunn@… added

comment:25 jeremyfelt3 months ago

  • Component changed from Administration to Login and Registration

comment:26 ircbot7 weeks ago

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

Note: See TracTickets for help on using tickets.