Closed
Bug 798669
Opened 12 years ago
Closed 12 years ago
nsImapMailFolder::GetOfflineMsgFolder should be made accessible to js
Categories
(MailNews Core :: Networking: IMAP, defect)
MailNews Core
Networking: IMAP
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 19.0
People
(Reporter: dlech, Assigned: atuljangra)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
Followup bug to item 9 in https://bugzilla.mozilla.org/show_bug.cgi?id=721316#c60 nsImapMailFolder::GetOfflineMsgFolder is not exposed in nsIMsgImapMailFolder.idl As a result, we don't have a way to write xpcshell tests to test if it is working right. Related source code: https://mxr.mozilla.org/comm-central/ident?i=GetOfflineMsgFolder&tree=comm-central&filter=
Assignee | ||
Comment 1•12 years ago
|
||
Do we really want to nsImapMailFolder::GetOfflineMsgFolder accessible to js? ( I am not against it or something, just confirming :) ) CC'ed Standard8.
Comment 2•12 years ago
|
||
Hmm, Atul has an interesting question. In some ways I could see this being tested via HasMsgOffline and GetOfflineFileStream - we can easily have test files for the profile, and then the unit test can try and get the stream, if the stream doesn't succeed, then obviously its not picking it up. On the other hand, I don't think it'd hurt to expose, as GetOfflineFileStream is exposed, and it may even be useful to extensions, though I can't think of a use case at the moment.
Assignee | ||
Comment 3•12 years ago
|
||
Added a method nsMsgDBFolder::GetOfflineMsgFolder, so that things go well in general. Also function is now accessible to idl file, thus exposed to extensions.
Attachment #675431 -
Flags: review?(mbanner)
Assignee | ||
Comment 4•12 years ago
|
||
Confirming the bug based on c#2
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → atuljangra66
Status: NEW → UNCONFIRMED
Ever confirmed: false
Comment 5•12 years ago
|
||
Comment on attachment 675431 [details] [diff] [review] First Version, just to get started Review of attachment 675431 [details] [diff] [review]: ----------------------------------------------------------------- The general concept is fine for me, so sr=me. Passing the actual review to irving. ::: mailnews/base/public/nsIMsgFolder.idl @@ +532,5 @@ > out long long aOffset, > out unsigned long aSize); > > /** > + * Get the folder where the msg could be present. nit: trailing whitespaces in several places (use the splinter review to see where these show up). @@ +534,5 @@ > > /** > + * Get the folder where the msg could be present. > + * @param msgKey key of the msg for which we are trying to get the folder; > + * @param[out] aMsgFolder required folder; We normally use @returns (as per the function below) @@ +537,5 @@ > + * @param msgKey key of the msg for which we are trying to get the folder; > + * @param[out] aMsgFolder required folder; > + * > + */ > + nsIMsgFolder GetOfflineMsgFolder(in nsMsgKey msgKey); You need to change the uuid for nsIMsgFolder. ::: mailnews/base/util/nsMsgDBFolder.cpp @@ +5969,5 @@ > + NS_IF_ADDREF(*aMsgFolder = this); > + return NS_OK; > + } > + } > + return NS_OK; nit: whitespace at end of line.
Attachment #675431 -
Flags: superreview+
Attachment #675431 -
Flags: review?(mbanner)
Attachment #675431 -
Flags: review?(irving)
Assignee | ||
Comment 6•12 years ago
|
||
Fixed the nits pointed out by Standard8. Now waiting for Irving's review. :-)
Attachment #675431 -
Attachment is obsolete: true
Attachment #675431 -
Flags: review?(irving)
Attachment #676298 -
Flags: review?(irving)
Comment 7•12 years ago
|
||
Comment on attachment 676298 [details] [diff] [review] Fixed the nits Review of attachment 676298 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/base/public/nsIMsgFolder.idl @@ +538,5 @@ > + * @returns aMsgFolder required folder; > + * > + */ > + nsIMsgFolder GetOfflineMsgFolder(in nsMsgKey msgKey); > + Trailing white space
Attachment #676298 -
Flags: review?(irving) → review+
Assignee | ||
Comment 8•12 years ago
|
||
Fixed everything, already got sr+ by Standard8 and r+ by irving. Good to go. :)
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 9•12 years ago
|
||
For the person who will be landing this: Please land the last attachment. Whitespace has been fixed in the last patch.
Comment 10•12 years ago
|
||
Comment on attachment 676298 [details] [diff] [review] Fixed the nits Please mark obsolete patches as such. It'll save you having to make comments like the last one :)
Attachment #676298 -
Attachment is obsolete: true
Comment 11•12 years ago
|
||
https://hg.mozilla.org/comm-central/rev/2cd71031e850 Also, for the future, bugs shouldn't be resolved until the patch actually lands.
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 19.0
Comment 12•12 years ago
|
||
Building on Windows is also a good thing to do. Backed out. https://hg.mozilla.org/comm-central/rev/7b51fd24e5bb https://tbpl.mozilla.org/php/getParsedLog.php?id=16874288&tree=Thunderbird-Trunk nsImapIncomingServer.cpp e:\builds\moz2_slave\tb-c-cen-w32\build\mailnews\imap\src\nsImapMailFolder.h(321) : error C2695: 'nsImapMailFolder::GetOfflineMsgFolder': overriding virtual function differs from 'nsMsgDBFolder::GetOfflineMsgFolder' only by calling convention e:\builds\moz2_slave\tb-c-cen-w32\build\objdir-tb\mozilla\dist\include\nsMsgDBFolder.h(57) : see declaration of 'nsMsgDBFolder::GetOfflineMsgFolder'
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
Updated•12 years ago
|
Target Milestone: Thunderbird 19.0 → ---
Assignee | ||
Comment 13•12 years ago
|
||
Thanks for the corrections Ryan :). I will keep this in mind.
Assignee | ||
Comment 14•12 years ago
|
||
Everything good to go. Tested on try server. Thanks Ryan for your help :-)
Attachment #679827 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 15•12 years ago
|
||
https://hg.mozilla.org/comm-central/rev/4c9774a9a2a7
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 19.0
Updated•12 years ago
|
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 16•12 years ago
|
||
Why was I marked as the reviewer for this patch? :/
Assignee | ||
Comment 17•12 years ago
|
||
Mike, I don't know :-/ I just saw it. Maybe Ryan knows the answer. :)
You need to log in
before you can comment on or make changes to this bug.
Description
•