Posted by stevepurkiss on January 7, 2013 at 8:16pm
12 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | install system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Problem/Motivation
The password strength indicator during the install process no longer displays in the correct place and only the text is displayed with no line/indicator.
Whilst working on #432962: Add option to disable password strength checking the display of the password strength indicator broke. Originally I thought it was to do with the work we were doing #432962-59: Add option to disable password strength checking however after trying an install without our patch it is still broken. It's as if the CSS just isn't there any more.
Proposed resolution
We are going to try and fix it by looking at what it does in D7 and what is now missing in D8.
Remaining tasks
Fixing
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| install-type.png | 85.65 KB | Ignored: Check issue status. | None | None |
Comments
#1
It looks as though there was no CSS at all for the password strength. In comparing the install form to the user edit form, user.css from core user.module was adding in the required styles for the password strength checking.
I'm not sure if this file was ever included in the install process, but adding it back to the form made the password strength checking work fine for me.
In install.core.inc, function _install_configure_form, I added the following
$form['#attached']['css'] = array(drupal_get_path('module', 'user') . '/user.css');I'd supply a patch but I'm not sure if the css was ever included from user.css, where it may have been, or why it may have been removed.
Adding in the entire contents of user.css had no detrimental effects on the rest of the page.
#2
Noticed this too... Seems at least normal priority (if not major); that floating word looks very ugly out there.
I think attaching the CSS is probably the right way to go but I wonder if we can track down what caused this regression in the first place. Maybe there already exists code somewhere that is still trying to add that CSS but failing for some reason.
#3
I recently did a git bisect, if someone wants to do that to find out which commit broke this. I was not that bad actually.. just a lot of install, test, wipe, install test... etc.
I followed the binary search section of: http://git-scm.com/book/en/Git-Tools-Debugging-with-Git
#4
related: #1811240: Improve "password matches" and "password strength" accessibility
#5
I'm pretty sure it broke with this commit:
http://drupalcode.org/project/drupal.git/commit/2bd155b1069fc631c1e1b0d0...
In that commit:
Removed from user.info:
stylesheets[all][] = user.cssReplaced with in user.module:
/*** Implements hook_page_build().
*/
function user_page_build(&$page) {
$path = drupal_get_path('module', 'user');
$page['#attached']['css'][$path . '/user.css'] = array('every_page' => TRUE);
}
This is equivalent for normal pages, but
hook_page_buildis not called for installation pages, because those are run through the maintenance page theme. Given this, it seems like using the form attachment is reasonable, but the correspondinguser.jsfile is actually loaded onto the installation page fromuser_library_info().user.jsis almost entirely about the password widget, so if that's being loaded throughuser_library_info(), I thinkuser.cssshould be, too. I've tested it, and it fixes the problem. (as does the form attachment method)Attached is a patch to add
user.csstouser_library_info().#6
Does what it says it should. :)
#7
Hooray! :) Thanks for tracking this down!
Committed and pushed to 8.x.
#8
Automatically closed -- issue fixed for 2 weeks with no activity.