Remove old error handling from theme_update_page()

Newbie Use-Case: I encountered this issue as well with no error messages on Alpha4. I saw screen refreshes with a generic error message with no hint as to why the install hadn't proceeded to the next phase. I thought I had already created my db when I actually hadn't. I had only tinkered with Drupal on D6 previously, and I had been spoiled by both the Bitnami Drupal stack and the Fantastico CPanel Drupal installer from my web-host, both of which prompted me for db set-up info and took care of the db creation business as part of the install.

I had never attempted a basic Drupal install before D7, and I glossed over the instructions telling me to first create the database. Perhaps I'm also accustomed to installers taking care of the creation of underlying databases on other software tools. I may have also had the impression that XAMPP or other such tools took care of the Drupal database install as well.

I don't know where this non-error fits in the big scheme of things, but it can be a frustrating first experience for Drupal Newbies. After 25 years in the industry, I've chosen Drupal and D7 as my next addiction, so I wasn't about to give up that easily. If I was just kicking the tires, I might have bailed and moved onto the next CMS.

I am also encountering this, however in my case the database and user have been created, I've done this many times with Drupal 6 on the same Linux server using the same tools and only in the case of Drupal 7 have I run into the problem of the installation not getting past the database configuration screen after triple checking the db and id/pw.

Of course not having any error messages makes this all the more frustrating. FWIW I was able to install D7 on my XAMPP box. I wonder if it may be related to the fact that the server I'm using has a prefix. I've tried including the prefix, removing the prefix, and adding the prefix onto the advanced options textfield. Obviously not having an error message makes this all guesswork and has gotten me nowhere.

Confirmed, this is imho critical and has worked before.

I'm looking into it.

Ok, I found something... Actually, a few things....

- First, form_set_error() does not seem to work during installing, it does add the error message to drupal_set_message() but not the form fields. This might be and old issue

- Then, there is a strange inconsistency between install_settings_form_validate and install_database_errors. If there is an exception thrown, the latter adds the exception message to $errors without an key but the previous function assumes that the key matches the name of the form field and uses that for form_set_error(). I first thought this is the main issue here, but it's not...

- Actually, it seems that template_preprocess_maintenance_page() eats the $variables['messages'] content defined in theme_install_page(). I added a check to only initialize it if it is not yet set. With that change, the error messages are now displayed, I also discovered an E_NOTICE error in theme_install_page(). The two above issues remain however, we might need to look into these too...

Nice fix, I think this is slightly cleaner though. Why are we using show_messages=FALSE when we actually do want it to show messages? Also use !isset() instead of empty() when initializing variables.

I think the attached patch would be even cleaner :)

As far as I can tell, the special handling of messages in the installer no longer has any purpose and can be removed. The "check the error messages and try again" link that it puts on this page doesn't really make sense there (the 'try again' link just has the effect of clearing out your form entirely!), nor does the special heading it adds, and there are other bugs this causes as well - see, for example, #753764: Duplicate and incorrect error messages during install. As far as I can tell, the only reason it was ever in the codebase to begin with was to serve as a kind of requirements warning early on in the installer - but this was way back in Drupal 5. Starting in Drupal 6, we've had a dedicated requirements check page with a nice task list, so this purpose doesn't seem to be relevant anymore, and now it is just getting in the way and causing other bugs.

Thoughts?

By the way, I haven't checked, but I wonder if update.php has the same problem? (Its theme function was changed recently at the same time this one was, and in a similar way.) If so, I think we could do the exact same thing in theme_update_page() as here.

#6 works great for me ... tried with bad db name and with wrong pw

Great catch, and great fix. Less code is always good. Committed to CVS HEAD.

I'm setting this to 'needs review' so we can investigate cleaning up theme_update_page() as well (per #6).

Lowering the priority and updating the title.

I confirmed that the bug exists for update.php also and that a similar fix works there. The fix is probably fine here too, since like the installer, update.php now has its own requirements page and task list which would seem to meet the goal this code was trying to achieve.

So, the attached patch makes theme_update_page() and theme_install_page() completely consistent with each other, and also removes some code comments elsewhere that incorrectly implied there was still some special template handling going on in these functions.

Lookss good. Committed to CVS HEAD. Thanks.

So those two functions have the same body now?

Just wondering if it is ok two have two functions with the same body.