Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#5762 closed patch (fixed)

Fix mail handling

Reported by: felipec Owned by: QuLogic
Milestone: 2.5.0 Component: MSN
Version: 2.4.1 Keywords:
Cc:

Description

Apparently the mail credentials need to be renewed each 777 seconds.

Attachments (3)

pidgin-felipec-42075c.diff (935 bytes) - added by felipec 11 years ago.
pidgin-felipec-6f8db9.diff (3.7 KB) - added by felipec 11 years ago.
pidgin-felipec-e80c23.diff (1.3 KB) - added by felipec 11 years ago.
Remove unnecessary initial url fetch.

Download all attachments as: .zip

Change History (16)

Changed 11 years ago by felipec

Changed 11 years ago by felipec

comment:1 Changed 11 years ago by felipec

Stu made some changes that fix issues for live.com accounts, but still the supported addresses are hard-coded, for example @microsoft.msn.com wouldn't work :)

Also, QNG is not a good measurement for timeouts, with the HTTP Method for example we don't recieve any QNG.

Changed 11 years ago by felipec

Remove unnecessary initial url fetch.

comment:2 Changed 11 years ago by felipec

With my other patches also it's no longer necessary to manually request the inbox url. It can be done at any time.

Also, this patch it demonstrates why it's bad to hard-code the "good" e-mail addresses.

comment:3 Changed 11 years ago by evands

  • Owner changed from khc to QuLogic

QuLogic, is this still relevant after the msnp15 merge? If so, could you look at it, please?

comment:4 follow-up: Changed 11 years ago by QuLogic

Well, it works for me, but that's right at login. I assume this is for after that?

42075c looks reasonable. I assume it's because "URL" is treated as async and thus cmd->trans isn't set for that callback. Aren't we sending a "URL", though? Should this not be treated as async, or is there a chance we could get it unrequested?

If the right time is 777 seconds, then why does the code say 750? Is that "just in case"?

e80c23 makes sense as long as the rest does.

That QNG result isn't used as a timeout anymore.

I will try to commit this tomorrow, as it's too late for me to test now.

comment:5 in reply to: ↑ 4 Changed 11 years ago by felipec

Replying to QuLogic:

Well, it works for me, but that's right at login. I assume this is for after that?

Right.

42075c looks reasonable. I assume it's because "URL" is treated as async and thus cmd->trans isn't set for that callback. Aren't we sending a "URL", though? Should this not be treated as async, or is there a chance we could get it unrequested?

I believe at some point it was sent automatically, maybe not anymore.

If the right time is 777 seconds, then why does the code say 750? Is that "just in case"?

Yes.

e80c23 makes sense as long as the rest does.

That QNG result isn't used as a timeout anymore.

I will try to commit this tomorrow, as it's too late for me to test now.

comment:7 Changed 11 years ago by qulogic@…

(In 5c5ae1b7efa28d9a5b76a5be3b0888149b8b0ab2):
Always save the MSN transaction in each command, not just for synchronous ones. Apparently, "URL" may or may not be async, but we also send it ourselves, too, and we need the transaction for other things.

References #5762.

comment:8 Changed 11 years ago by qulogic@…

(In 5e33eda76b698c04377f5ad6cdd3ad1ec87d2eff):
Fixup MSN mailbox handling based on a patch from Felipe, plus various other things that I cleaned up in the URL command handler. Updates are now made after at least 750 seconds because Felipe found that to be the magic value, and updating every time we receive a QNG is totally unreliable. They aren't even received when using the HTTP method, for example.

Opening the inbox is now always available. I'm not sure why it was limited to just @hotmail.com and @msn.com, but I certainly haven't been testing with either of those. I think the correct way to determine if an inbox exists is to just use the URL command and see, but I don't have one of those no-inbox accounts.

The initial email notification is no longer called explicitly for @hotmail.com and @msn.com accounts. I, at least, get an initial mail notification, but the rest of Felipe's patch negates the need to do this, anyway.

References #5762.

comment:6 Changed 11 years ago by QuLogic

Hmm, I thought that last change might break initial e-mail notification, but it looks like it still works. It seems like we get two initial_mdata_msg() calls for some reason, so passport_info.file gets created the first time, then used the second time. If that changes at MSN's end, this will need to be fixed. Maybe I'll look when I have time later.

I'm going to leave this open for a little while (the commit's somewhere, assuming trac manages to find it) until I can look into that, but I assume you've had this in msn-pecan for a while already, so I don't foresee any big problems arising.

comment:7 Changed 11 years ago by felipec

That's why msn-pecan doesn't have a physical file, all that can be written in the url, no need to have a file.

comment:8 Changed 11 years ago by QuLogic

You could have mentioned that earlier...

comment:9 Changed 11 years ago by felipec

There are many things msn-pecan does differently, you are free to check the changes of email notification:

git log --pretty=oneline notification.c | grep mail

Using a temporal file will always have issues.

comment:12 Changed 11 years ago by qulogic@…

(In 337eed6a3abf681c14acf327e6f8f5cc088cbd4b):
Use "EmailEnabled?" from the MSN profile message to determine whether the email inbox can be opened.

References #5762.

comment:12 Changed 11 years ago by qulogic@…

  • Milestone set to 2.5.0
  • Resolution set to fixed
  • Status changed from new to closed

(In ac4ea8dd7d88b80021aa72a43b686f2bdc5514d3):
Use a URL to open MSN Hotmail inbox instead of a temporary file. Based on work by felipec in msn-pecan.

Fixes #5762.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!