This is a continuation of comment #9 in http://drupal.org/node/204191. NaX posted a code clean-up patch there with the following description:

The attached patch takes some of your last few cleanup commits a small step further.
It firstly fixes the dialog CSS to work in older IE versions and I would just like to say I really like the new default look of the dialog, its theme independent.
Secondly I moved the output from the user_auth function to its own function and then used it in user_auth.
Once this was done I made the password reset message use drupal messages and the dialog.

I've attached his patch.

CommentFileSizeAuthor
#1 contrib-5.securesite.clean-up.junyor.patch2.77 KBjunyor
securesite-clean-up.nax_.patch4.74 KBjunyor

Comments

junyor’s picture

junyor commented
StatusFileSize
new contrib-5.securesite.clean-up.junyor.patch2.77 KB

Here's a patch updated for the DRUPAL-5 branch. I still need to review it, but now it at least applies cleanly.

junyor’s picture

junyor commented
Status: Needs review » Needs work

OK, some comments:

* The function name securesite_user_login() isn't really appropriate. The login actually takes place in securesite_init(). securesite_user_auth() currently does both processing and shows the auth dialog. If you want to factor out the latter, I'd call the new function securesite_user_auth() and change the name of the existing securesite_user_auth().
* Make sure to update the function documentation. The comments before securesite_user_auth() still says it shows the auth dialog, which isn't correct after this patch.
* Should we really call securesite_user_login() after sending a mail? I put the exit there so the user will see that a mail has been sent. We could try to do something so they see the message, then they could click a link to initiate login again.
* Which version(s) of IE needed the changes to the dialog stylesheet?
* Your original post said you "made the password reset message use drupal messages and the dialog", but I don't see that in the patch. Am I missing something?

BTW, I'm glad you like the new dialog. I wasn't sure what others thought.

junyor’s picture

junyor commented

I'm not sure your idea to use drupal_set_message() would work. Wouldn't the HTTP auth dialog in securesite_user_login() show, instead of the message? That's why there's an exit after the mail is sent.

Which versions of IE don't work with the new dialog? I'm pretty sure I checked IE6, though I can't check at the moment.

junyor’s picture

junyor commented
Status: Needs work » Fixed

I tweaked the HTML dialog code a bit and tested in IE6, IE7, Opera 9.x, Safari 3.1, Firefox 2, and Firefox 3. It seems to be working pretty well.

I also changed the code so the HTML dialog is used after sending password mails instead of just outputting text.

Checked in on the DRUPAL-5 branch.

NaX’s picture

NaX commented

I am sorry that I did not participate in this issue. I have just not been available for the last while. I had a look at your commit and it looks fine. The main idea behind my suggested changes was to output valid html. Your way is simpler, without changing too much.

The older browser that I was referring to was IE5.5. I have an old machine running 2000 and I some times use it to test things. From what I can tell, you can’t style the body element the way you are trying. 2000 is an old OS and the dialog is fully themeable, so it’s no big deal.

@Junyor - Thanks for all the hard work.

junyor’s picture

junyor commented

OK. Unfortunately, I don't have access to IE 5.5.

You're welcome. :) I'm glad I finally found a module I can contribute to in a meaningful way.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.