Duplicate "try again" links when themes_api fails

[solarissmoke]

When the Themes API fails, for example when searching for themes (in my case because of timed out connection to wp.org, two "Try again" links are presented to the user.

The issue is with this line in WP_Theme_Install_List_Table->prepare_items()

wp_die( $api->get_error_message() . '</p> <p class="hide-if-no-js"><a href="#" onclick="document.location.reload(); return false;">' . __( 'Try again' ) . '</a>' );

$api->get_error_message() already returns a try again link ( set by themes_api() ), but this line adds another one in addition. I suggest removing the latter? Also, there are no CSS rules of .hide-if-no-js on Error pages, so it doesn't do anything.

[nacin]

.hide-if-no-js is in global.dev.css.

Is this is a regression?

[solarissmoke]

Replying to nacin:

.hide-if-no-js is in global.dev.css.

But the error page only links to wp-admin/css/install.css (see _default_wp_die_handler() ) - I think this has always been the case.

Also, my original suggestion that we should remove the second "try again" link is wrong, because themes_api can return two different errors. I've changed the patch. As far as I can see this shouldn't negatively affect other calls to themes_api().

[solarissmoke]

Don't print duplicate "try again" links when themes_api fails

[SergeyBiryukov]

Replying to nacin:

Is this is a regression?

Yep. Tested on wp-admin/theme-install.php?tab=featured screen: 3.0.4 didn't use wp_die() here and there was only one “Try Again” link.

[nacin]

(In [17264]) Leave the 'Try Again' link for the wp_die() when the themes API fails. props solarissmoke, fixes #16132.

[nacin]

Spoke with dd32 who added the original code, and he agreed that removing it from the API abstraction layer would be a better choice, and keep it with the wp_die().

[solarissmoke]

Hmm, I think this needs more work. Now, when themes_api fails we get "An Unexpected HTTP Error occurred during the API request." but not the error from wp_remote_post() which actually has the more meaningful information about what went wrong.

Knocking up a patch now.

[solarissmoke]

... and show more useful error information

[solarissmoke]

Alternatively, we might do away with the "unexpected HTTP error" bit altogether (it isn't particularly helpful) and just pass on the wp_remote_post error:

$res = new WP_Error( 'themes_api_failed', $request->get_error_message() );

[dd32]

IMO, We leave it as-is for the 3.1 release and revisit in 3.2 with the intention of cleaning up some of the update ui's

[westi]

Replying to dd32:

IMO, We leave it as-is for the 3.1 release and revisit in 3.2 with the intention of cleaning up some of the update ui's

I agree.

@solarissmoke please can you create a new ticket for the improves and add your patch to it.

Thanks.

[solarissmoke]

Yep will do