User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.7.12) Gecko/20050915 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.7.12) Gecko/20050915 When a user clicks on the "Forgot my password" link, he is taken to the login page, which does include the form to request the forgotten password, but as the last option. The URL does contain an anchor to this section, but the page is too small for this to have any effect for most users. So the user begins to read the page, and wonders why he has to login with the password he has forgotten. Reproducible: Always Steps to Reproduce: 1. On the bugzilla home page, click on "Forgot my password" Actual Results: A page that does include the form to request a new password, but as the last item of the page Expected Results: In this case, the form to request a new password is the most proeminent item of the page.

We can probably improve the UI a bit, I agree ;)

Make the 'forgot' anchor into a parameter in login-small. When the parameter is set, put the forgot password form first in the login page.

Use a BLOCK instead of duplicating code.

Comment on attachment 212497 [details] [diff] [review] patch v1.1 >Index: Bugzilla/Auth/Login/WWW/CGI.pm Do not edit this file. There is no reason to do so. >Index: template/en/default/account/auth/login.html.tmpl This UI doesn't make sense IMO. If the user is has forgotten his password, the UI should only show the form to enter his login name. The form to enter his login and password should be hidden.

We are in "soft freeze" mode to prepare 3.0 RC1.

Bugzilla 3.2 is now frozen. Only enhancements blocking 3.2 or specifically approved for 3.2 may be checked in to the 3.2 branch. If you would like to nominate your enhancement for Bugzilla 3.2, set "blocking3.2" tp "?", and either the target milestone will be changed back, or the blocking3.2 flag will be granted, if we will accept this enhancement for Bugzilla 3.2.

Comment on attachment 359709 [details] [diff] [review] Patch V2 > [% PROCESS global/variables.none.tmpl %] >+[% USE CGI %] No, there's a "cgi" object available in every template that you can use. >+[% IF CGI.param('forgot') != 1 %] Should be "cgi" >+ style = "#forgot{ margin: 2ex 5em 5ex 1em; width: 60ex}" _ >+ "label{ font-weight: bold;}" _ >+ "#request{ margin-top: 1em;}" Put that in a CSS file somewhere, instead of here. >+[% IF CGI.param('forgot') != 1%] You should be checking NOT cgi.param('forgot') instead of doing it that way. Also, it's better to make booleans positive. So you should check cgi.param('forgot') and then do an ELSE, unless this is much easier. >+ <th align="right"><label for="Bugzilla_login">Login:</label></th> While we're here, it'd be nice to do that alignment by CSS in a CSS file somewhere instead of in the HTML. >+ (Note: you should make sure cookies are enabled for this site. >+ Otherwise, you will be required to log in frequently.) I don't think we need that anymore. Almost everybody has cookies enabled nowadays. >+[% ELSE %] > [% IF user.authorizer.can_change_password %] Any particular reason not to just always show this, but just not show the above if forgot=1? >+ <h3> Forgot Password </h3> >+ <form method="get" action="token.cgi" id="forgot" name="forgot"> This form should POST, because it does something. >+ [% terms.Bugzilla %] does not allow users to change their own passwords. That's somewhat misleading--it makes it sound like we don't support the feature. Perhaps "This Bugzilla"? >Index: bugzilla/template/en/default/global/common-links.html.tmpl >+ [% script_name = script_name.replace("[&]?forgot=1","") %] Man, modifying the query string with a regex is a bad idea. Can we just do this somewhere else, like calling cgi.delete or something?

You know what? I had to use the "Forgot password" feature today (with JS disabled), and I again wondered why it took me to the login form. IMO, this is so confusing that I would propose to take it for 4.0, because I consider this buggy behavior more like a bug than an enhancement. mkanat, would you agree to take it for 4.0?

Well, I'm not particularly interested in the usability of Bugzilla when it doesn't have JavaScript enabled--I'm only concerned that it simply works at all. So I don't think this would be a high-priority item for 4.0 or for any release.

(In reply to comment #14) > all. So I don't think this would be a high-priority item for 4.0 or for any > release. I'm not saying it's a high priority. I'm talking about taking it for 4.0.

Okay. No, I wouldn't want to take it for 4.0.

Comment on attachment 528030 [details] [diff] [review] v1 Review of attachment 528030 [details] [diff] [review]: Overall, it seems like the "forgot" page and the "log in" page are now so different that they should actually just be separate templates. You should check for "forgot" inside of the code and render the right template as appropriate. A few code comments also: ::: template/en/default/account/auth/login.html.tmpl @@ +31,2 @@ [% PROCESS global/variables.none.tmpl %] +[% IF Bugzilla.cgi.param('forgot') != 1 %] Instead of doing "!=" here, do == and reverse the two blocks. @@ +40,5 @@ +[% END %] + +<style > + form label { font-weight: bold; } + title = "Forgot Password" This whole <style> block should probably be in a CSS file somewhere. @@ +44,5 @@ + #bugzilla-body form table tr th { vertical-align: middle; } + #forgot { width: 60ex} + #request { margin-top: 1em;} +</style> +[% END %] This block becomes really long, perhaps it should become a BLOCK that gets processed. (Also, same note as above--make the boolean check positive if possible.)

We are going to branch for Bugzilla 4.4 next week and this bug is too invasive or too risky to be accepted for 4.4 at this point. The target milestone is set to 5.0 which is our next major release. I ask the assignee to reassign the bug to the default assignee if you don't plan to work on this bug in the near future, to make it clearer which bugs should be fixed by someone else on time for 5.0.

Dylan, can you confirm that this bug is still valid in the current code base? If so, could you relink to where the source is, what file to edit and add a mentor to the bug?

Here is the new login form with patch v2 (attachment 8567752 [details] [diff] [review]).

glob said on IRC he would prefer the "Email Address" label to not wrap.

Oops, I didn't want to paste the whole window.

Comment on attachment 8568553 [details] [diff] [review] patch, v2.1 Review of attachment 8568553 [details] [diff] [review]: ----------------------------------------------------------------- I would prefer request-new-password.html.tmpl go in the template/en/default/account/auth directly like the others. ::: template/en/default/pages/request-new-password.html.tmpl @@ +21,5 @@ > + </p> > + > + <form id="forgot" method="get" action="token.cgi"> > + <input type="hidden" name="a" value="reqpw"> > + <input size="35" name="loginname" required> nit: add type="email" if not Param('emailsuffix'), otherwise type="text".

Comment on attachment 8574279 [details] [diff] [review] patch, v3 Review of attachment 8574279 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. r=dkl

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git c3b984a..cd00796 master -> master

Comment on attachment 8574279 [details] [diff] [review] patch, v3 >+++ b/createaccount.cgi >-$user->check_account_creation_enabled; test_create_user_accounts.t detected that Bugzilla was letting the user enter his email address before throwing an error that account creation is disabled. If account creation is disabled, an error should be thrown immediately, as it did in Bugzilla 5.0 and older. We cannot leave this check at the top of createaccount.cgi, because we still want to allow existing users to change their password if they forgot it. To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git dc3e779..b57e5e3 master -> master