Make WordPress Core

Opened 12 years ago

Last modified 5 years ago

#20716 assigned enhancement

Control how and when request_filesystem_credentials outputs creds form

Reported by: griffinjt's profile griffinjt Owned by: dd32's profile dd32
Milestone: Priority: normal
Severity: normal Version: 3.4
Component: Filesystem API Keywords: has-patch needs-refresh
Focuses: Cc:


When using WP_Filesystem in a plugin or other external app, you can initialize it this way:

if ( false == ( $creds = request_filesystem_credentials( $url, $method, false, false, $form_fields ) ) ) {
    return true;

Makes sense. If the method isn't direct or we don't have the credentials we need, request_filesystem_credentials outputs a form and returns false. Then we check again to make sure the credentials can be verified, and it not, we continue to output the form until the credentials are good.

if ( ! WP_Filesystem( $creds ) ) {
    request_filesystem_credentials( $url, $method, true, false, $form_fields );
    return true;

request_filesystem_credentials arbitrarily outputs the form whether you want it to or not, unless you explicitly filter the request_filesystem_credentials function itself. Using this in conjunction with Ajax, you have to use output buffering to catch the output and return it to the Ajax script for processing or else you get errors. This definitely isn't ideal.

I'd suggest storing the form in a variable and possibly pass a parameter that determines whether or not we want to actually output the form or not. Or, if there is another (better) way, that's cool too. Just some way to help request_filesystem_credentials determine how we want to interact with credentials should we need them.

Attachments (1)

20716.diff (1.2 KB) - added by jeremyfelt 11 years ago.

Download all attachments as: .zip

Change History (9)

#1 @griffinjt
12 years ago

Err, should be === instead of == in that first function. Typo.

#2 @dd32
11 years ago

  • Keywords needs-patch added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 3.7

This is needed for #22704 specifically for [25228]

Anyone got a patch? :)

11 years ago

#3 @jeremyfelt
11 years ago

  • Keywords has-patch added; needs-patch removed

I think 20716.diff addresses the issue here. It introduces an output_request_filesystem_credentials filter that defaults to true. If set to false, then it returns false for request_filesystem_credentials() rather than outputting the form.

If any of the output is needed for some reason, then a different approach would be needed. We could also throw in a DOING_AJAX check so that the filter doesn't need to be set for some requests.

#4 @nacin
11 years ago

  • Owner set to dd32
  • Status changed from new to assigned
  • Type changed from enhancement to task (blessed)

#5 @dd32
11 years ago

It seems that we'd be better to split most of the logic before the form into a separate function, which request_filesystem_credentials() calls. We can then stop calling request_filesystem_credentials() directly, and call that helper instead.

#6 @johnbillion
11 years ago

  • Milestone changed from 3.7 to Future Release

Punting as per dev chat and dd32's comment.

#7 @chriscct7
9 years ago

  • Keywords needs-refresh added

#8 @ocean90
8 years ago

  • Type changed from task (blessed) to enhancement
Note: See TracTickets for help on using tickets.