WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#22187 closed enhancement (fixed)

Add docblocks to wp-signup.php

Reported by: DrewAPicture Owned by: SergeyBiryukov
Milestone: 3.6 Priority: normal
Severity: minor Version: 3.4
Component: Inline Docs Keywords: has-patch commit needs-codex
Focuses: Cc:

Description

It's my understanding that wp-signup.php has sort of fallen by the wayside in the recent past and could use some love. By no stretch of my imagination do I think I know all there is to know about what needs to happen here, but doesn't hurt to get the ball rolling.

Some action items I can already suggest would be:

  • Make it possible to overload the hard-coded CSS
  • Add some docblocks for the many functions in the file
  • Cleanup coding standards and optimize strings
  • ???

Attachments (6)

22187-signup-styles.diff (1.3 KB) - added by DrewAPicture 3 years ago.
22187.2.diff (6.3 KB) - added by DrewAPicture 3 years ago.
Docblocks for all functions + changes from 22187-signup-styles.diff
22187.3.diff (25.4 KB) - added by DrewAPicture 3 years ago.
22187.2.diff + general cleanup of the entire file
22187.4.diff (25.9 KB) - added by DrewAPicture 3 years ago.
Add some missing semicolons causing WSOD
22187-docblocks.diff (7.0 KB) - added by DrewAPicture 3 years ago.
Just the docblocks
22187-docblocks.2.diff (6.9 KB) - added by DrewAPicture 3 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 @DrewAPicture3 years ago

  • Cc xoodrew@… added
  • Keywords has-patch added

Howdy from #WCDEN Hack Day, I thought I'd try to work on the lower-level bits today.

A conversation in IRC with @dd32 this morning led to 22187-signup-styles.diff which I think should easily allow themes to overload the hard-coded styles. I'm still not totally clear on the reasoning behind adding or not adding actions/filters to default-filters.php.

comment:2 @DrewAPicture3 years ago

Related: #17166 - wp-signup dynamic strings to static strings

@DrewAPicture3 years ago

Docblocks for all functions + changes from 22187-signup-styles.diff

comment:3 @toscho3 years ago

  • Cc info@… added

@DrewAPicture3 years ago

22187.2.diff + general cleanup of the entire file

comment:4 @DrewAPicture3 years ago

Attached 22187.3.diff which:

  • Allows for overloading the hard-coded CSS
  • Adds first-pass docblocks to all the functions
  • General coding standards and code clarification through the whole file

TODO: Clarify docblocks, look at moving some of the functions out to ms-functions.php

@DrewAPicture3 years ago

Add some missing semicolons causing WSOD

comment:5 @DrewAPicture3 years ago

Some more related tickets:

  • #17630 - Redirect wp-signup.php to wp_signup_location
  • #19451 - Cannot distinguish which WordPress Signup stage I am
  • #17948 - Enhancements to the login and registration forms
  • #16833 - Signup mechanism shortens usernames without warning
  • #20107 - Add Filter for user_name length in ms user signup

@DrewAPicture3 years ago

Just the docblocks

comment:6 @DrewAPicture3 years ago

  • Component changed from Multisite to Inline Docs
  • Severity changed from normal to minor
  • Version set to trunk

The OCD in previous aside, 22187-docblocks.diff just adds docblocks to wp-signup.

Version 0, edited 3 years ago by DrewAPicture (next)

comment:7 @DrewAPicture3 years ago

  • Milestone changed from Awaiting Review to 3.6

comment:8 follow-up: @SergeyBiryukov3 years ago

Some text needs updating:

The default styles are printed via signup_header which is hooked to wp_head.  
They can be overloaded by replacing the output on the signup_header action hook. 

This was the case in 22187.4.diff, but not in 22187-docblocks.diff.

comment:9 in reply to: ↑ 8 @DrewAPicture3 years ago

  • Summary changed from Clean up wp-signup.php to Add docblocks to wp-signup.php

Replying to SergeyBiryukov:

Good catch Sergey. 22187-docblocks.2.diff should cover it. See anything else that stands out?

comment:10 @SergeyBiryukov3 years ago

  • Keywords commit added; dev-feedback removed
  • Version changed from trunk to 3.4

comment:11 @SergeyBiryukov3 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 23362:

Add inline documentation for wp-signup.php. props DrewAPicture. fixes #22187.

comment:12 @SergeyBiryukov2 years ago

As noted in ticket:24210:11, $meta value passed to confirm_blog_signup() and confirm_another_blog_signup() is an array, not a string (although it's unused in both functions):
tags/3.5.1/wp-signup.php#L209, tags/3.5.1/wp-signup.php#L343.

Also, there's an inconsistency in optional arguments. In confirm_blog_signup(), both $user_name and $user_email are optional. In confirm_another_blog_signup(), $user_name is required. Neither of them make sense as optional there, but that's out of scope for this ticket.

comment:13 @SergeyBiryukov2 years ago

In 24262:

Correct default value and inline description for $meta argument. see #22187.

comment:14 @DrewAPicture2 years ago

  • Keywords needs-codex added

See [23362] and [24262] for phpdoc updates to:

  • do_signup_header()
  • wpmu_signup_stylesheet()
  • show_blog_form()
  • validate_blog_form()
  • show_user_form()
  • validate_user_form()
  • signup_another_blog()
  • validate_another_blog_signup()
  • confirm_another_blog_signup()
  • signup_user()
  • validate_user_signup()
  • confirm_user_signup()
  • signup_blog()
  • validate_blog_signup()
  • confirm_blog_signup()

comment:15 @toscho2 years ago

Looks good, thanks. I am not so sure about the @since MU – shouldn’t that be a real version number (3.0)?

comment:16 @SergeyBiryukov2 years ago

@since MU is not the same as @since 3.0.0, it's used for functions that actually come from the MU codebase. The difference is specifically mentioned in ticket:16702:1.

Note: See TracTickets for help on using tickets.