Closed
Bug 436792
Opened 16 years ago
Closed 16 years ago
Incorrect RDF Data Source function prototypes following nsISupportsArray patch in bug 435290 (IsCommandEnabled, DoCommand)
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9
People
(Reporter: standard8, Assigned: rain1)
References
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
13.77 KB,
patch
|
Details | Diff | Splinter Review | |
9.74 KB,
patch
|
rain1
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
Compiling with the latest trunk I'm seeing the following errors when compiling nsMailModule.cpp: /Users/moztest/mozilla/hg/mozilla/mailnews/build/../base/src/nsMsgRDFDataSource.h:68: warning: ‘virtual nsresult nsMsgRDFDataSource::IsCommandEnabled(nsISupportsArray*, nsIRDFResource*, nsISupportsArray*, PRBool*)’ was hidden /Users/moztest/mozilla/hg/mozilla/mailnews/build/../base/src/nsMsgFolderDataSource.h:117: warning: by ‘virtual nsresult nsMsgFolderDataSource::IsCommandEnabled(nsIArray*, nsIRDFResource*, nsIArray*, PRBool*)’ /Users/moztest/mozilla/hg/mozilla/mailnews/build/../base/src/nsMsgRDFDataSource.h:68: warning: ‘virtual nsresult nsMsgRDFDataSource::DoCommand(nsISupportsArray*, nsIRDFResource*, nsISupportsArray*)’ was hidden /Users/moztest/mozilla/hg/mozilla/mailnews/build/../base/src/nsMsgFolderDataSource.h:121: warning: by ‘virtual nsresult nsMsgFolderDataSource::DoCommand(nsIArray*, nsIRDFResource*, nsIMutableArray*)’ nsMsgFolderDataSource inherits via nsMsgRDFDataSource from nsIRDFDataSource.idl I'm not sure if we really make use of DoCommand and IsCommandEnabled, but its not a good idea to change those function prototypes. Seeing as we're not going to able to change nsIRDFDataSource (core code), it looks as if we're going to have to back these bits out from bug 435290?
Flags: blocking-thunderbird3.0a2?
Assignee | ||
Comment 1•16 years ago
|
||
Unfortunately everything down the stack from there uses the same arrays, so pretty much the whole bug will have to be backed out. I'm pretty sure nothing in JS uses the RDF commands anymore. I'm looking this up.
Comment 2•16 years ago
|
||
Should fix this for a2.
Flags: blocking-thunderbird3.0a2? → blocking-thunderbird3.0a2+
Assignee | ||
Comment 3•16 years ago
|
||
They almost certainly aren't used any more.
Assignee: nobody → sid1337
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•16 years ago
|
||
To elaborate, local and mxr greps on the entire mail/ and mailnews/ folders produced no results which were relevant. There is a separate DoCommand in nsMsgDBView.cpp, http://mxr.mozilla.org/mozilla/source/mailnews/base/src/nsMsgDBView.cpp#2231 , with a different function signature, and this is the only one that's used. grep: http://mxr.mozilla.org/mozilla/search?string=DoCommand&find=%2Fmail&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla
Assignee | ||
Updated•16 years ago
|
Attachment #323308 -
Flags: review?(bugzilla)
Comment 5•16 years ago
|
||
Did you actually *check* that they aren't used anymore? The whole command stuff is core XUL code and so mxring only mailnews and mail does sound a bit frightening...
Assignee | ||
Comment 6•16 years ago
|
||
This is a conservative patch that reverts to nsISupportsArray, but copies data to nsIMutableArrays wherever required (well, just one place it seems). Either this patch or the other should be checked in. Karsten: I'm slightly confused. Do you think that this code might be accessed in other places through the nsIRDFDataSource interface?
Attachment #323308 -
Attachment is obsolete: true
Attachment #323315 -
Flags: review?(bugzilla)
Attachment #323308 -
Flags: review?(bugzilla)
Assignee | ||
Updated•16 years ago
|
Attachment #323308 -
Attachment is obsolete: false
Attachment #323308 -
Flags: review?(bugzilla)
Reporter | ||
Comment 7•16 years ago
|
||
Comment on attachment 323308 [details] [diff] [review] removed DoCommand, IsCommandEnabled and related functions I think given that we're working on removing rdf, and the uncertainty over being sure this isn't required, we should go for the conservative patch.
Attachment #323308 -
Flags: review?(bugzilla)
Reporter | ||
Comment 8•16 years ago
|
||
Comment on attachment 323315 [details] [diff] [review] Conservative patch + // Create an nsIMutableArray from the nsISupportsArray + nsCOMPtr<nsIMutableArray> folderArray(do_CreateInstance(NS_ARRAY_CONTRACTID)); + for (PRUint32 i = 0; i < itemCount; i++) + { + folderArray->AppendElement(arguments->ElementAt(i), PR_FALSE); + } You don't need the braces around this. r=me with those removed.
Attachment #323315 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 9•16 years ago
|
||
(carrying forward r=Mark)
Attachment #323315 -
Attachment is obsolete: true
Attachment #323683 -
Flags: review+
Assignee | ||
Updated•16 years ago
|
Attachment #323683 -
Flags: superreview?(bienvenu)
Comment 10•16 years ago
|
||
Comment on attachment 323683 [details] [diff] [review] Update to conservative patch, addressing comment yes, I agree.
Attachment #323683 -
Flags: superreview?(bienvenu) → superreview+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 11•16 years ago
|
||
Checking in mailnews/base/src/nsMsgFolderDataSource.cpp; /cvsroot/mozilla/mailnews/base/src/nsMsgFolderDataSource.cpp,v <-- nsMsgFolderDataSource.cpp new revision: 1.223; previous revision: 1.222 done Checking in mailnews/base/src/nsMsgFolderDataSource.h; /cvsroot/mozilla/mailnews/base/src/nsMsgFolderDataSource.h,v <-- nsMsgFolderDataSource.h new revision: 1.85; previous revision: 1.84 done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•