authorize.php connection settings form broken (doesn't degrade, pointless fieldset)

~Subscribing! :)

Maybe rather than trying to hack in a fix for this, we just simplify to the same mechanism the installer uses when selecting which database to use? (i.e., install_settings_form from includes/install.core.inc)

If you've never installed Drupal without JS enabled, a screenshot of the page is attached to this post.

Found the following errors in the form submission process:

1. Function definition of authorize_filetransfer_form($form_state) is wrong. It should be authorize_filetransfer_form($form, &$form_state) instead.>
2. Button #attributes class should be an array, not a string.
3. authorize_filetransfer_form_validate checks the connection on all stages of form input, not just the final stage (after all connection information has been gathered).

Fixing these three problems results in a working non-JS interface.

Attached is a patch, and screenshots of the workflow in a non-JS client. (The final success page is not shown, as it is the same for a JS client). Workflow for JS client is unchanged.

Issues:

1. "WARNING: You are not using an encrypted connection, so your password will be sent in plain text." sometimes appears twice, but I don't know how to fix this one. Using drupal_set_message is probably wrong.
2. Button placement of "Change Connection Type" is ugly. (See attached screenshots).

Recommendation:
Commit this patch since it restores functionality for non-JS clients. Then downgrade to a normal bug and work out the remaining (visual) kinks.

Redid the warning message to be a form element instead of drupal_set_message:

-    drupal_set_message(t('WARNING: You are not using an encrypted connection, so your password will be sent in plain text. <a href="@https-link">Learn more</a>.', array('@https-link' => 'http://drupal.org/https-information')), 'error');
+    $form['information']['https_warning'] = array(
+      '#prefix' => '<div class="messages warning">',
+      '#markup' => t('WARNING: You are not using an encrypted connection, so your password will be sent in plain text. <a href="@https-link">Learn more</a>.', array('@https-link' => 'http://drupal.org/https-information')),
+      '#suffix' => '</div>',
+    );

(The string was not changed).

Visually, the change is shown in the attached image.

webchick: I think this is ready to go... I've thoroughly tested it at this point on clients with both JS enabled and disabled.

Tried to install a module with this patch applied, with JS disabled in browser. It worked as explained. Also tried to install a module with JS enabled, and it worked like before. I could see nothing visibly broken.

It could use some cleanup, but at least now it's functional. Cleanup can be done later, in my opinion, as it would be great to have this in before release, to avoid unnecessary frustrations.

Works for me. :) I like the change from 'fieldset' to 'container'. Not so sure about losing the header text --- I think it's okay, the information is certainly there elsewhere.

There might be some complaints that changing from FTP to SSH erases the settings, but this is by-design. (Having the header text there made this change clearer, I think).

Looks great :). Presenting the WARNING as an error is fine with me, and as dww pointed out, a previous issue.

I like keeping the header. It's not the prettiest at the moment, but I think it leaves the door open for further usability improvements since the string will be available.

OMG status mess up!

Edit: I did actually apply this patch and verify that installation procedure works for both JS enabled and disabled, and the screenshots presented by dww in #14 are correct as well.

#14: 932846-14.fix-authorize-php-no-js-no-nested-fieldset.patch queued for re-testing.

Setting to RTBC -- the bot failed because

FAILED: [[SimpleTest]]: [MySQL] Setup environment: failed to clear checkout directory.

Looks good, this had UX sign off months ago - so get it in, I'd say

We're missing the most important screenshot of them all --- successfully installing a module when JS is disabled! :) Good work all!

+++ includes/authorize.inc	4 Jan 2011 00:19:19 -0000
@@ -184,6 +192,12 @@ function _authorize_filetransfer_connect
+  if($form_state['clicked_button']['#name'] != 'process_updates') {

Missing space after the if. Watch to see if it gets fixed in the actual commit...

Also, maybe should use '#limit_validation_errors' => array(), instead of the above, or change the '#submit' on some of the buttons. Or whatever, this will have to do for now.

Test bot is borked.

#24: 932846-24.fix-authorize-php-no-js-no-nested-fieldset.patch queued for re-testing.

Awesome work. This is a really great issue to get crossed off the list before 7.0!

Committed to HEAD.