Make WordPress Core

Opened 13 years ago

Last modified 21 months ago

#15729 accepted enhancement

Better UX after supplying incorrect information to setup-config

Reported by: nacin's profile nacin Owned by: kapeels's profile kapeels
Milestone: Future Release Priority: low
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch needs-refresh
Focuses: Cc:

Description

In #15682 I added a 'Try Again' button to step 2 of setup-config.php if the credentials were incorrect.

The button is a simple link with a javascript:history.go(-1) hack. This should instead be a form with hidden values that submits back to setup-config.php?step=1, and fills out the form so they can try again.

Additional enhancement: If the prefix is malformed (can only contain letters, numbers and underscores), we do a wp_die(). We should again have a form with a 'Try Again' button that returns them.

As an added bonus, incorrect credentials should be filled out (for reference) but marked as incorrect and potentially with the focus set on that field.

Only caveat I can think of: This will require the use of esc_attr(), which we don't have access to yet. Not sure how to solve that without reverting to htmlspecialchars() with some extra work.

Attachments (6)

15729-2.diff (6.5 KB) - added by kapeels 13 years ago.
15729-3.diff (6.5 KB) - added by kapeels 13 years ago.
Forgot not to use heredoc and nowdoc. This one doesn't.
15729-4.diff (7.3 KB) - added by kapeels 13 years ago.
15729-6.diff (7.5 KB) - added by kapeels 13 years ago.
15729-7.diff (7.8 KB) - added by kapeels 13 years ago.
15729-8.diff (7.8 KB) - added by kapeels 13 years ago.
This has got to be commitable now :D

Download all attachments as: .zip

Change History (21)

#1 @kapeels
13 years ago

  • Cc kapeel.sable@… added
  • Owner set to kapeels
  • Status changed from new to accepted

#2 @kapeels
13 years ago

I'm on it for GCI.

#3 in reply to: ↑ description @kapeels
13 years ago

Replying to nacin:

As an added bonus, incorrect credentials should be filled out (for reference) but marked as incorrect and potentially with the focus set on that field.

Do you mean that the fields have been left incomplete? I can't find a way to determine which field has been wrongly filled.

Focus can be set, but, how to "mark" them as incorrect? By changing border colors to red?

#4 @nacin
13 years ago

You're right, it's tough. There's no way to tell if it's user, server, or password. But there is a differentiation between these three:

  • user, host, password
  • database name
  • table prefix

In lieu of border colors, let's just try a simple error message at the top in <p> tags, with three different error messages (one for each case). "Cannot check to the database server with this username and password.", "Cannot select this database.", "The table prefix can contain only letters, numbers, and underscores."

Then, focus the first applicable field via injected JS, either name, user, or table prefix.

@kapeels
13 years ago

@kapeels
13 years ago

Forgot not to use heredoc and nowdoc. This one doesn't.

#5 @kapeels
13 years ago

  • Cc kapeel.sable@… removed
  • Keywords has-patch 2nd-opinion added; needs-patch removed

#6 @nacin
13 years ago

  • Keywords 2nd-opinion removed

Minor thing, but watch for trailing whitespace.

Take a look at our coding standards: http://codex.wordpress.org/WordPress_Coding_Standards. Function names should be written with underscores separating the words.
I think we can avoid a new function all together. That would also avoid the overuse of globals here. What we can do is create our own WP_Error object, then modify the branching a bit to allow the DB connection to only be tested if the prefix is valid, and either way, we can then show a form to send them back to. It doesn't even need to be a WP_Error object
, just something we can keep track of.

For both hidden elements and in the form inputs, we need to properly escape these. We don't have access to esc_attr() here, so a call to htmlspecialchars( $string, ENT_QUOTES ) sh
ould do. Rather than just doing this for the prefix. Also, always escape as late as possible -- so in the form in both cases.

I think we're close. Nice work so far.

@kapeels
13 years ago

@kapeels
13 years ago

#7 @nacin
13 years ago

  • Keywords 3.2-early added

Last patch looks great. :-)

Final feedback:

  • Not sure why display_header() was un-indented. It should be one indent inside of the case.
  • Rename the 'passwrd' varto 'password' or 'pwd'. It looks like a typo.
  • Initialize setup_error as a new WP_Error. Use ->add() inside the preg_match. Remove the setup_error == null check and make that into an else for the preg_match.

@kapeels
13 years ago

@kapeels
13 years ago

This has got to be commitable now :D

#8 @SergeyBiryukov
13 years ago

Perhaps we should add the charset parameter to all instances of htmlspecialchars(). Though I'm not exactly sure if MySQL allows UTF-8 characters in database name, username or password, I've tried to create a MySQL user with Cyrillic username and it seems to be working.

Last edited 13 years ago by SergeyBiryukov (previous) (diff)

#9 @SergeyBiryukov
11 years ago

The button is a simple link with a javascript:history.go(-1) hack. This should instead be a form with hidden values that submits back to setup-config.php?step=1, and fills out the form so they can try again.

This would resolve #24836.

Additional enhancement: If the prefix is malformed (can only contain letters, numbers and underscores), we do a wp_die(). We should again have a form with a 'Try Again' button that returns them.

Related: #19970

#11 @chriscct7
9 years ago

  • Keywords needs-refresh added; gci 3.2-early removed

#14 @pento
5 years ago

  • Keywords bulk-reopened added

#15 @andraganescu
5 years ago

  • Milestone set to Future Release
  • Type changed from defect (bug) to enhancement

#16 @andraganescu
5 years ago

The enhancement seems like a good idea indeed. The patch just needs a refresh.

#17 @desrosj
21 months ago

  • Keywords bulk-reopened removed
Note: See TracTickets for help on using tickets.