Project

General

Profile

Bug #12108

Password reset styling/scripts don't work correctly

Added by Boone Gorges almost 2 years ago. Updated almost 2 years ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
WordPress (misc)
Target version:
Start date:
2019-11-18
Due date:
% Done:

0%

Estimated time:

Description

When you go through the password-reset process beginning at https://commons.gc.cuny.edu/wp-login.php?action=lostpassword&error=invalidkey, the next step (after getting the key) requires you to enter a new password. This interface has a password reset strength meter. But the scripts required for this UI to work - jquery, user-profile, etc - are not loaded properly. This is a result of the header-juggling in wp-content/mu-plugins/cac-login-template.php; see #3780.

I tried some tricks to make them work - force-enqueuing scripts, etc - but everything I tried either doesn't work or is very hacky (like harvesting all the script tags from the original header and dumping them into the new header). It's like WP clears out the enqueue stack in order to prevent jQuery from loading twice, or something like that. Ray, could you have a look and see whether you've got any ideas? Worst case, we could probably manually put the relevant script tags in the login header, but I'm unsure of the ramifications of this across themes.

Screenshot_2019-11-18_16-43-26.png (75.4 KB) Screenshot_2019-11-18_16-43-26.png Boone Gorges, 2019-11-18 06:22 PM
password-strength.png (183 KB) password-strength.png Raymond Hoh, 2019-11-22 02:06 AM

Related issues

Related to CUNY Academic Commons - Bug #6644: White Screen at Login PgeReporter Feedback2016-11-08

Related to CUNY Academic Commons - Bug #3780: wp-login.php template stylingResolved2015-02-02

History

#2 Updated by Raymond Hoh almost 2 years ago

  • Category name set to WordPress (misc)
  • Target version set to 1.15.14

Thanks for the report, Boone.

I'll take a look at it before the next maintenance release.

#3 Updated by Raymond Hoh almost 2 years ago

  • Related to Bug #6644: White Screen at Login Pge added

#4 Updated by Raymond Hoh almost 2 years ago

  • Related to Bug #3780: wp-login.php template styling added

#5 Updated by Raymond Hoh almost 2 years ago

So this took me down the rabbit hole!

In #3780, we customized the login page so it would use the current theme's page template. That technique involved object-buffering the contents of the login header first, followed by the current theme's page template and later injecting some login markup into the page template.

The rationale for this, as far as I can remember, was to not include various unrelated assets (like plugin CSS and JS) on the login page.

I did remember a similar issue that was raised awhile ago -- #6644. #6644 found a quirk with how this technique is used and Boone found the symptoms, but I guess I totally forgot about that ticket!

So anyway, I believe the quirk is due to how WordPress internally queues up various scripts and assets. Their queue system is only meant for the current page. However, since the page template technique involves technically loading two pages, this confuses WordPress.

I've addressed this by reversing the order of how I am juggling the template buffering. I'm now loading the current theme's page template first, followed by the login header. This fixes the jQuery problem that Boone mentioned in the main ticket description. The second fix to allow late enqueueing involves bringing back WP's footer script rendering function into the login page.

Once I did those two things, I enqueued the password reset scripts that WP uses as well as added some inline CSS to make it all look good. See attached screenshot.

Here are the list of changes: https://github.com/cuny-academic-commons/cac/compare/115e542...5789789

Boone, one other thing to note is the admin bar assets were being included again due to the reverse buffering order, so I removed those assets manually. I don't believe we ever had a discussion on whether we wanted the admin bar to be rendered on the login page. If we want to do that, let's open a new ticket to discuss that.

#6 Updated by Boone Gorges almost 2 years ago

Nice sleuthing, Ray! You're a better man than me - I could see that the two-pageload + script-enqueuing system were the culprits, but I didn't have the wherewithal to figure out a workaround :)

Boone, one other thing to note is the admin bar assets were being included again due to the reverse buffering order, so I removed those assets manually. I don't believe we ever had a discussion on whether we wanted the admin bar to be rendered on the login page. If we want to do that, let's open a new ticket to discuss that.

Yup, let's leave the toolbar out for now. Thanks!

#7 Updated by Boone Gorges almost 2 years ago

  • Status changed from Staged for Production Release to Resolved

Also available in: Atom PDF