Closed Bug 79245 Opened 23 years ago Closed 23 years ago

Offline: Shouldn't prompt to send Unsent if there aren't unsent messages.

Categories

(SeaMonkey :: MailNews: Backend, defect, P2)

All
Other
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.5

People

(Reporter: laurel, Assigned: Bienvenu)

References

Details

(Whiteboard: #79865 has the combined patch, PDT+)

Attachments

(6 files)

Using may 07 commercial trunk build. When going from Offline state back to Online I'm getting asked if I want to send unsent messages when there is nothing to send. I believe 4.x didn't prompt if the Unsent Messages folder was empty. 1. Go to mail login to (IMAP) account. 2. Go to Local Folders/Unsent Messages, make sure it is empty. 3. File|Work Offline. (I didn't download anything... simple case.) 4. File|Work Online. Result: Prompt appears about sending Unsent messages when going online. Expected: Nothing, no prompt since I have no unsent messages.
QA Contact: esther → gchan
front end
Assignee: bienvenu → dianesun
OK, a special check needs to be added in to check the msg # of the Unsent folder in order to decide if prompt the UI. Reassign to Mohanb.
Assignee: dianesun → mohanb
Keywords: nsbeta1
*** Bug 83029 has been marked as a duplicate of this bug. ***
Priority: -- → P2
Target Milestone: --- → mozilla0.9.2
Mohan, are most of these diff whitespace diffs? If so, you can do a diff -uw to eliminate the whitespace diffs and just show the relevant changes. If that's the case, could you attach a new diff? Thanks!
Attached patch patch with "diff -uw" (deleted) — Splinter Review
In check for unsent messages, you should break out of that loop if you find that any identity has unsent messages. Also, why are you adding brace style like the following:? if (a) { do something } that's pretty non-standard, and seems not needed either.
Attached patch proposed patch v2 (deleted) — Splinter Review
PDT+ per 6/12 mtg.
Whiteboard: [PDT+]
Whiteboard: [PDT+] → [PDT+] requesting r & sr
cc'ing bhuvan for review & sspitzer for super review;
I can review this to unload bhuvan some... :) + allIdentities = am.allIdentities; Does |allIdentities| already exist, or did you forget to declare it with var? + var msgFolder = new Object(); I'm having a FUD issue with this line 'o code... Is this really needed? Remember that we're inside a for-loop when doing this... It might be expensive, memory-wise. + if (msgFolder.value) { .value? What is the check here meant to do? Check if msgFolder is non-null? + var numMessages = msgFolder.value.getTotalMessages(false); is value really a part of the XPCOM object, so getTotalMessages() is declared in the value property of msgFolder? Sorry if I'm asking dumb questions, but I don't understand some parts of your code.
Attached patch patch v2.1 (deleted) — Splinter Review
+ allIdentities = am.allIdentities; var declared. + var msgFolder = new Object(); kept out of the loop. + if (msgFolder.value) { .value? To check whether msgFolder has got proper value from GetUnsentMessagesFolder? + var numMessages = msgFolder.value.getTotalMessages(false); Yes, getTotalMessages() is declared in the value property of msgFolder?
+ var msgFolder; [...] + msgFolder = new Object(); Make that |var msgFolder = new Object();| . + numMessages = msgFolder.value.getTotalMessages(false); Sorry, I still don't understand this one. getTotalMessages() is a function of the nsIMsgFolder interface - nothing else. For an example look at /mailnews/base/resources/content/mail3PaneWindowCommands.js, line 955. One further question is, since that code obviously should not work - does it even work for you? Did you test your fix with multiple accounts and unsent messages in your unsent folder?
Attached patch patch v2.2 (deleted) — Splinter Review
---------------- 1. I am passing a new Object() to msgSendlater along with current Identity; 2. msgFolder.value is an "nsIMsgFolder" interface; 3. /mailnews/base/resources/content/mail3PaneWindowCommands.js, line 955 is an "nsIMSgFolder" interface obtained from RDF; 4. I think if you look at GetUnsentMessagesFolder(currentIdentity, returnMsgFolder) for the msgSendlater it will be more clear; 5. look at commandglue.js : 822 to know how to pass a out parameter to the function getFoldersWithFlag(MSG_FOLDER_FLAG_TRASH, 1, out); and usage of out.value; 822 var out = new Object(); 823 var trashFolder = rootFolder.getFoldersWithFlag(MSG_FOLDER_FLAG_TRASH, 1, out); 824 numFolder = out.value; ----------------
r=hwaara. Note to the super-reviewer of this code: please check if it's just me on crack regarding the .value thing. I still don't think it should be there.
Whiteboard: [PDT+] requesting r & sr → [PDT+] requesting sr
Whiteboard: [PDT+] requesting sr → [PDT+] requesting sr Planning for checkin by FRI : 06/22
Whiteboard: [PDT+] requesting sr Planning for checkin by FRI : 06/22 → [PDT+] requesting sr
Who is the sr= person for this bug? Please use all means to ensure they know you've made the request.
seth, can I get sr=;
does this patch even work? it looks like it would give you a js error. you added a function CheckForUnsentMsgs(), but you call CheckForUnsentMessages() in one place. please double check that, and attach a new patch. (please do cvs diff -uw so that white space changes don't show up.) + if(CheckForUnsentMessages()) offlineManager.goOnline(true, true, msgWindow); + else + offlineManager.goOnline(false, true, msgWindow); break could become: + offlineManager.goOnline(CheckForUnsentMsgs(), true, msgWindow); while you do that (and finish testing) I'll continue reviewing.
+ please do cvs diff -uw so that white space changes don't show up. whoops, ignore that comment.
# 79865 has the combined patch with these changes;
Whiteboard: [PDT+] requesting sr → [PDT+] #79865 has the combined patch
so you need a sr of bug #79865? that bug doesn't have a r= yet.
Blocks: 79480
pushing out. 0.9.2 is done. (querying for this string will get you the list of the 0.9.2 bugs I moved to 0.9.3)
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Removing PDT+, marking nsbranch; should be checked into the 0.9.3 trunk ASAP, tested & verified, then submitted for limbo builds.
Keywords: nsBranch
Whiteboard: [PDT+] #79865 has the combined patch → #79865 has the combined patch
Fix checked in on behalf on mohanb.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Adding vtrunk; bugs need to be verified in the trunk for eventual limbo checkins.
Status: RESOLVED → ASSIGNED
Keywords: vtrunk
Clearing resolution, nsBranch bugs should be closed only after checkins into limbo builds.
Resolution: FIXED → ---
Commercial builds 2001-07-05-09-trunk/ win nt 4.0 2001-07-05-08-trunk/ linux 2.2, red hat 7.0 2001-07-05-08-trunk/ mac os 9.0.4 There is still a problem. What's weird it is only a WINDOWS prob. 1.works fine if you initially start messenger click the offline icon. Then click the icon again to go online as there is no message prompt to send unsent messags. Doesn't work for this case: 1. Go offline 2. compose a message and do send later 3. go online 4. prompt to send unsent messages apears, say send. 5. mesg is sent. 6. go offline 7. go back online 8. the unsent messages prompt pops up even though there are no unsent messages in my unsent folder Tried the same test case on mac and linux. no problem. it only appears on windows. tried multiple new profiles and I see this. The windows build has id of 2001070504 and using NT 4.0
Missed 0.9.3.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Reassigning 0.9.4 Offline IMAP UI bugs to David; TFV 0.9.5.
Assignee: mohanb → bienvenu
Status: ASSIGNED → NEW
Blocks: 99230
Jussi/David - Is this worthy of nsenterprise+ or nsbranch+. If not, please minus this one.
I tried this - it worked fine. I think we need Gary to retest this.
Status: NEW → ASSIGNED
using commercial branch build 2001091203 on nt 4.0 and 2k i still see this problem David. I did not check this on linux or mac. After going offline, compsing a mesg, doing send later, going back online, clicking send unsent mesg button on the window prompt, then going back offline, then back online I still see the prompt to send unsent mesgs. Also a side note, my unsent messages folder is stiil showing 1 message though when I click on the folder, I don't see any messages in that folder. This might be a outliner problem and possibly not even related to this bug?
Are you sure the message gets sent? It seems like it might not be, which would explain why we prompt again. I tried those exact same steps and it worked fine, like I said. What's the name of your unsent messages folder? Is your sent folder online or local?
my unsent messages folder still displays a count of 1 after it sends, so that is an outliner problem, or more accurately, the send unsent messages code is not updating the counts correctly. Do you select the unsent messages folder at some point in this process, or are the steps as general and simple as you describe?
Ok. I'll try to answer your questions David. First I'm using the commercial TRUNK build now 2001091203 on nt 4.0 I've tried migrated profiles and brand new profiles. Using Modern theme. -Yes my unsent messages do get sent before I click offline icon again. in fact when I say yes to send unsent messages, I click the 'get mesg' button to ensure I get the messages before going back offline. -Name of my unsent messages folder is the default name: 'Unsent Messages' and the folder is located under the "Local Folders" account. -Yes I have selected the Unsent folders while I'm offline as I want to see message 'dissapear' when i go back online. But I have tested without ever clicking on the unsent messages folder and still see the same problem -My sent folder is set to the default online sent folder. I did try switching sent folder to local folders and I still see the same problem. I've tried a test mail server, seth's mail server, my judge mail account and i've seen it on all 3. My steps to reproduce are really that simple (using a brand new profile) 1.create new profile 2. start browser 3.start messenger 4.set up imap mail account 5.login to your mail account. 6.go offline 7.compose mesg and do a send later. 8.go back online 9. say 'send unsent messages' at the prompt 10.login to your outgoing mail server 11.click get mesg 12.verify the mesg has arrived 13.go back offline (click offline icon in messenger) 14.go back online (click offline icon in messenger) 15.The send unsent messages prompt does indeed pop back up. If I quit the application entirely and then restart browser and messenger,login, and click icon to go offline,then back online, the message prompt DOES NOT pop up. So it pops up only after actually sending a usent mesg and then going back online/offline/online/offline, etc.. Hope this helps and I am clear enough.
Comment on attachment 49166 [details] [diff] [review] part of fix - default prefs should have escaped uri's sr=mscott
Attachment #49166 - Flags: superreview+
we'd like to take this low risk fix for the branch.
Keywords: nsbranchnsbranch+
Comment on attachment 49166 [details] [diff] [review] part of fix - default prefs should have escaped uri's r=sspitzer
Attachment #49166 - Flags: review+
fix checked into trunk - I'm not 100% sure this is the problem you're seeing, Gary, but it's pretty likely.
No longer blocks: 99230
Blocks: 99508
PDT+ per PDT meeting today. Gary has signed off to do the QA on the branch.
Whiteboard: #79865 has the combined patch → #79865 has the combined patch, PDT+
David, using commercial 2001-09-14-09-trunk on nt 4.0, I am still seeing this problem. Tried old profile/new profile, judge email account, one of my tst mail accounts, and I see same problem. Tried it on linux trunk build and no problem. i'll try some other window platforms and see if it's NT 4.0 specific?
MScott - Ready to check-in.
Using 2001-09-17-09-trunk/ on nt 4.0, win 2k (2 computers), win 98, and win me, I think this is fixed. I tried new profiles/existing profiles and it does look like it is fixed. Once in a while, I can generate the problem and when that happens if I go to Usent messages folder under Local Folder account and right click and do a compact folder, it solves the problem. Number of messages in Usent folder goes from 1 to 0. I don't know if that lastlittle tidbit helps out or not.
fix checked into branch as well.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
Using commercial branch builds (9.4) 20010925 nt 4.0, linux 2.2, mac 9.1 Verified that the 'send unsent mesgs' prompt does not appear if your usent messages folder is empty. Tested both migrated and new profiles. Still need to verify on trunk.
commercial trunk 2001-10-08-09-trunk NT 4.0 2001-10-08-08-trunk linux 2.2 2001-10-08-08-trunk mac 9.1 Verified no prompt appears, if Unsent folder is empty, when going from offline to online. Tried migrated/new profiles. removed keyword vtrunk marking as verified
Status: RESOLVED → VERIFIED
Keywords: vtrunk
this bug is still exist,when our qa team,test this bug,mozilla aways have one unsend mail, although it was sended when mozilla online.but if i delete unsend folder file under localfolder,then mozilla will ok!
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: