[Fix proposal] Subscription widget form disappears after error

Hey guys, thanks for the great work on jetpack, it’s helped me do a lot of things really elegantly over the last few weeks! I’ve noticed a couple of bugs that I managed to fix, can this be added to the main branch?

1. Form disappears when the submission returns an error
If the form submit returns an error (ex: wrong email address format was entered; can’t reach wordpress.com for whatever reason; etc.), it displays a message like “There was an error when subscribing. Please try again.”. However, it doesn’t display the form anymore! It’s basically telling you to try again but without a chance to do so. Super frustrating.

here’s the code responsible for this, in modules/subscriptions.php, l637 to l666:

<?php
if ( ! isset ( $_GET['subscribe'] ) ) {
	?><div id="subscribe-text"><?php echo wpautop( str_replace( '[total-subscribers]', number_format_i18n( $subscribers_total['value'] ), $subscribe_text ) ); ?></div><?php
}

if ( $show_subscribers_total && 0 < $subscribers_total['value'] ) {
	echo wpautop( sprintf( _n( 'Join %s other subscriber', 'Join %s other subscribers', $subscribers_total['value'], 'jetpack' ), number_format_i18n( $subscribers_total['value'] ) ) );
}

if ( ! isset ( $_GET['subscribe'] ) ) { ?>
	<p id="subscribe-email">
		<label id="jetpack-subscribe-label" for="<?php echo esc_attr( $subscribe_field_id ); ?>">
			<?php echo !empty( $subscribe_placeholder ) ? esc_html( $subscribe_placeholder ) : esc_html__( 'Email Address:', 'jetpack' ); ?>
		</label>
		<input type="email" name="email" required="required" class="required" value="<?php echo esc_attr( $subscribe_email ); ?>" id="<?php echo esc_attr( $subscribe_field_id ); ?>" placeholder="<?php echo esc_attr( $subscribe_placeholder ); ?>" />
	</p>

	<p id="subscribe-submit">
		<input type="hidden" name="action" value="subscribe" />
		<input type="hidden" name="source" value="<?php echo esc_url( $referer ); ?>" />
		<input type="hidden" name="sub-type" value="<?php echo esc_attr( $source ); ?>" />
		<input type="hidden" name="redirect_fragment" value="<?php echo $widget_id; ?>" />
		<?php
			if ( is_user_logged_in() ) {
				wp_nonce_field( 'blogsub_subscribe_'. get_current_blog_id(), '_wpnonce', false );
			}
		?>
		<input type="submit" value="<?php echo esc_attr( $subscribe_button ); ?>" name="jetpack_subscriptions_widget" />
	</p>
<?php }?>

The problem comes from the fact that we check for form submission to display the form, and not submission success. As a result, even when there’s an error, it’s seen as “submitted” so it doesn’t display the form anymore.
To fix it, I updated the conditions to also check for the case where the form is submitted with a state different than “success” (because there are many kinds of errors):

<?php
if ( ! isset ( $_GET['subscribe'] ) OR $_GET['subscribe'] != 'success' ) {
	?><div id="subscribe-text"><?php echo wpautop( str_replace( '[total-subscribers]', number_format_i18n( $subscribers_total['value'] ), $subscribe_text ) ); ?></div><?php
}

if ( $show_subscribers_total && 0 < $subscribers_total['value'] ) {
	echo wpautop( sprintf( _n( 'Join %s other subscriber', 'Join %s other subscribers', $subscribers_total['value'], 'jetpack' ), number_format_i18n( $subscribers_total['value'] ) ) );
}

if ( ! isset ( $_GET['subscribe'] ) OR $_GET['subscribe'] != 'success' ) { ?>
	<p id="subscribe-email">
...

Solved the problem for me.

2. Hash navigation auto scrolling is broken
When validating the form, the user is directed to the same page with, along the url parameters, a hash like #blog_subscription-2. This is supposed to make the user automatically be scrolled to the form section of the page, so that he can see the result of his submission. However it doesn’t work at all, here’s why:

line 636, where the id of the form is set

<form action="#" method="post" accept-charset="utf-8" id="subscribe-blog-<?php echo $widget_id; ?>">

line 658, where the hash navigation is added

<input type="hidden" name="redirect_fragment" value="<?php echo $widget_id; ?>" />

The id’s don’t match. One is $widget_id and the other is “subscribe-blog-” + $widget_id. The hash navigation doesn’t match any id on the page and the browser can’t scroll the user nowhere.

Solution, make redirect fragment match actual form id :
line 658 becomes

<input type="hidden" name="redirect_fragment" value="subscribe-blog-<?php echo $widget_id; ?>" />

Now the user is scrolled to the form automatically after submitting.

Let me know if this is enough to be added to the main codebase (and hopefully I can just update my plugin and not have to hack on top of it), or if I need to do anything more to make it happen.

Cheers

https://ww.wp.xz.cn/plugins/jetpack/