Customizer theme install getting stuck on downloading

[helen]

I've been getting into situations with the new theme browser/installer in the customizer where it gets stuck on the downloading spinner or the install button doesn't actually work at all - you can click it but nothing happens, and no errors in the console.

I *think* this has to do with the fact that I'm also messing around with forcing the FTP credential prompt for other purposes and toggling that on and off in PHP without reloading the page. This itself may (probably) be an edge case that doesn't need addressing, but I think the error handling could be better and may come up in other situations.

Steps to reproduce:

1. Browse .org themes in the customizer.
2. With forced FTP on (will add code for this in a followup comment), attempt to "install and preview" from the grid view.
3. Observe that failure notice appears over the screenshot and button changes to Installation failed but remains primary and active
4. Disable forcing FTP in PHP elsewhere; do not reload page.
5. Button on the attempted theme is still clickable but does nothing, all other buttons will take you to the downloading spinner which never completes.

[helen]

@ocean90's plugin code for forcing FTP:

<?php
/**
 * Plugin Name: Force FTP
 * Description: Used to test FTP credentials screen.
 */

add_filter( 'filesystem_method', function() {
	return 'ftpext';
} );

add_filter( 'fs_ftp_connection_types', function() {
	return [
		'ftp'  => __( 'FTP' ),
		'ftps' => __( 'FTPS (SSL)' ),
		'ssh'  => __( 'SSH2' ),
	];
} );

[westonruter]

I was able to reproduce this a couple times when I was hacking around trying to force an unable_to_connect_to_filesystem error when calling wp_ajax_install_theme(). I think it has something to do with wp.updates.maybeRequestFilesystemCredentials(), and the wp.updates.ajaxLocked and wp.updates.shouldRequestFilesystemCredentials flags. I think it assumes a prompt is being shown to enter credentials, but I've never seen this appear. Maybe it is actually being shown but it is hidden behind the Customizer UI thanks to z-index issues?

[celloexpressions]

My expectation has been that the calls to wp.updates in the themes panel JS (and also maybe in the section JS) would show the filesystem credentials and handle related errors (with the adjustments to updates.js in #37661). It would be most helpful if the people who worked on shiny updates (I think @obenland or @swissspidy, possibly others?) could take a look at how it's being applied to the customizer. My guess is that the required error handling and credential overlays are mostly implemented but require minor tweaking to be fully functional in this context.

[obenland]

This seems to be an issue of a missing wp_print_request_filesystem_credentials_modal(); call in wp-admin/customize.php to capture FTP credentials. Once that was added, the theme browser should also listen for when the modal closes prematurely and return to the selection instead of the loading spinner.

[westonruter]

@obenland thanks for that! That makes sense. @helen Would you test 42184.0.diff​ to see if it fixes the issue you reported?

Note that you'll need to have that plugin active to force FTP mode before loading the Customizer, since the template will skip rendering if it detects it has direct write access to the filesystem. You wouldn't be able test with dynamically enabling/disabling FTP mode while the Customizer is open.

[westonruter]

I just noticed #42205 (FTP credentials dialog gets hidden behind things) which seems closely related.

[westonruter]

The problem is going deeper than just adding the credential modal template in 42184.0.diff​. When I add it, I do get the modal as expected in credential-modal.png​. But when I cancel out of that modal, I then get some weird behavior as can be seen in after-credential-modal-cancel.png​.

I think it may have to do with this credential-modal-cancel event handler running in the unexpected context of the Customizer: ​https://github.com/WordPress/wordpress-develop/blob/850db3c6a05af31efdd4ec982536cae4c496ae61/src/wp-admin/js/updates.js#L1780-L1825

In addition the buttons both getting the “Install” label, then when I try clicking on them they do nothing.

[celloexpressions]

Add handling for credential cancelling in the customize context.

[celloexpressions]

42184.1.diff​ should take care of most of the issues here, by correctly resetting labels and closing the downloading notification when the credentials modal is cancelled.

There are a couple of additional issues that I could not track down. The button you pressed breaks if you cancel out of the modal, but all of the other buttons still work. The labels can also be incorrectly assigned when the theme details modal is open (but not in the grid view). The aria-label is not reset properly, which may be because any aria-labels are handled differently in the admin context.

The best solution is probably to rethink the integration with updates.js entirely, not changing buttons or showing visible loading indicators from there in the customizer because the full-page notification is shown. Much of what was added in updates.js undoes what happens there for wp-admin. It's a bit late to do that for 4.9, but something to consider for the future. For now, 42184.1.diff​ makes most of this functional, and if anyone else can dig into the lingering issues further, that would be helpful.

[helen]

I'm not set up to test fully (actually entering credentials and having them work) but 42184.1.diff​ does at least allow the modal to appear the first time you press the install button for a given theme. As noted, the button ceases to work after that.

Also, just want to be clear that I doubt this is a release blocker, though it is pretty bad to have something that can make the user feel like they broke it.

[westonruter]

After dismissing the file credentials modal, I'm still seeing the same thing as after-credential-modal-cancel.png​, where both buttons say “Install”.

[celloexpressions]

Fix theme download error handling.

[celloexpressions]

#42314 was marked as a duplicate.

[celloexpressions]

Replying to westonruter:

After dismissing the file credentials modal, I'm still seeing the same thing as after-credential-modal-cancel.png​, where both buttons say “Install”.

Known issue noted in 11. The problem is that updates.js doesn't address multiple buttons with the same action; I don't see a minimally-hacky way to fix this without more broadly refactoring the integration. However, this only happens when a model is open, so it will be rarely encountered.

42184.2.diff​ fixes the failed theme download error handling following the new notifications API, which is something that users will run into much more frequently (for example, an intermittent internet connection or downtime on .org). The buttons still stop working for that, but they do show a notification failed error. It seems like it would be worth more broadly evaluating the error handling here, likely in a future (potentially minor) release so that there's time to consider all error cases to handle and refactor things at a higher level.

Summary of issues fixed in 42184.2.diff​:

• Theme download error handling restored following the use of a notifications API for the downloading overlay.
• Introduce preliminary customize-specific logic for the modal credential cancel handler.
• Ensure that scripts for the credentials modal are correctly enqueued.

Are there any other issues here that seem fixable for 4.9? Would be good to get the progress into beta 4, at least, for broader testing.

[afercia]

With 42184.2.diff applied I still see something not working correctly, to reproduce:

• force FTP
• customizer > change theme > .org themes
• click on any theme "Install & Preview" button
• the FTP credentials modal opens
• close the modal
• click on the same theme previously clicked "Install & Preview" button
• nothing happens

[westonruter]

Since SFTP is broken I think what we should do is disable installation entirely when SFTP is required. We can disable the install buttons and a notification to the Themes panel in the same way we do when the user has drafted or scheduled a changeset.

[westonruter]

Disable theme install and delete when FTP credentials required

[westonruter]

In 41997:

Customize: Prevent theme installation and deletion in Customizer while SFTP credentials need to be requested.

This is a temporary measure while we wait for credentials to be able to be supplied in the Customizer.

Amends [41788].
See #42184, #37661, #42126.

[westonruter]

Punting now that we temporarily disabled requesting for SFTP credentials in the Customizer.

[westonruter]

In 42018:

Customize: Remove theme_installing notification when installation fails.

Amends [41648].
Props celloexpressions.
See #42184, #37661.

[celloexpressions]

Next steps:

• Revert [41997]
• Use 42184.2.diff​
• Fix the two known bugs: incorrect labels after cancelling the modal, and fix broken buttons after cancelling the modal

Note that with 42184.2.diff​, SFTP appears to work as expected. The only issues are with canceling the modal, which happens much more in testing than in practice. [41997] seems like an overreaction to the minor bugs that we need to track down with that, which may seem worse than they are because they are more apparent in a testing workflow than in real usage. Either way, the fixes for that should not be particularly difficult. I'm not planning on working on this issue further, so someone else can take it on.

[westonruter]

Remaining issues to be addressed in #42873.

[westonruter]

In 42536:

Customize: Prevent showing SFTP theme installation notification in multisite when user cannot install themes.

Props flixos90.
See #37661, #42184.
Fixes #42674.

[westonruter]

In 42537:

Customize: Prevent showing SFTP theme installation notification in multisite when user cannot install themes.

Props flixos90.
See #37661, #42184.
Fixes #42674 for 4.9 branch.