Closed
Bug 312241
Opened 19 years ago
Closed 18 years ago
nsIconURI::Clone and ::Resolve are unimplemented
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: chpe, Assigned: chpe)
References
Details
(Keywords: fixed1.8.0.2, fixed1.8.1, Whiteboard: [nvn-dl])
Attachments
(2 files, 1 obsolete file)
1.06 KB,
patch
|
Biesinger
:
review+
bryner
:
superreview+
benjamin
:
approval-branch-1.8.1+
mtschrep
:
approval1.8.0.1-
dveditz
:
approval1.8.0.2+
|
Details | Diff | Splinter Review |
2.58 KB,
patch
|
chpe
:
review+
bryner
:
superreview+
|
Details | Diff | Splinter Review |
http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/decoders/icon/nsIconURI.cpp#434 : 434 NS_IMETHODIMP 435 nsMozIconURI::Clone(nsIURI **result) 436 { 437 return NS_OK; 438 } 439 440 NS_IMETHODIMP 441 nsMozIconURI::Resolve(const nsACString &relativePath, nsACString &result) 442 { 443 return NS_OK; 444 } They should throw NS_ERROR_NOT_IMPLEMENTED, but I'd really like at least ::Clone to be implemented.
Assignee | ||
Comment 1•19 years ago
|
||
Assignee | ||
Updated•19 years ago
|
Attachment #199359 -
Flags: superreview?(bryner)
Attachment #199359 -
Flags: review?(cbiesinger)
Updated•19 years ago
|
Attachment #199359 -
Flags: review?(cbiesinger) → review+
Updated•19 years ago
|
Attachment #199359 -
Flags: superreview?(bryner) → superreview+
Comment 2•19 years ago
|
||
Checking in modules/libpr0n/decoders/icon/nsIconURI.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/icon/nsIconURI.cpp,v <-- nsIconURI.cpp new revision: 1.20; previous revision: 1.19 done
Comment 3•19 years ago
|
||
Can this bug now be RESOLVED FIXED?
Assignee | ||
Comment 4•19 years ago
|
||
(In reply to comment #3) > Can this bug now be RESOLVED FIXED? I still want at least ::Clone to be really implemented; attachment 199359 [details] [diff] [review] was just the minimal fix.
Assignee | ||
Updated•19 years ago
|
Summary: nsIconURI::Clone and ::Resolve return NS_OK but don't do anything → nsIconURI::Clone and ::Resolve are unimplemented
Comment 5•19 years ago
|
||
I think filing another bug for that would be the right approach. This one seemed to be about NS_OK being returned when it should really have been NS_ERROR_NOT_IMPLEMENTED; which has now been fixed.
Comment 6•19 years ago
|
||
Or maybe i'm totally wrong and should shut up :)
Comment 7•19 years ago
|
||
Comment on attachment 199359 [details] [diff] [review] the easy way: throw NS_ERROR_NOT_IMPLEMENTED Requesting approval1.8.0.1 and approval1.8.1 for the patch because biesi says it will fix the crash in bug 320621.
Attachment #199359 -
Flags: approval1.8.1?
Attachment #199359 -
Flags: approval1.8.0.1?
Comment 8•19 years ago
|
||
Comment on attachment 199359 [details] [diff] [review] the easy way: throw NS_ERROR_NOT_IMPLEMENTED Please consider for 1.8.1 - 1.8.0.1 is for major security and crash issues only.
Attachment #199359 -
Flags: approval1.8.0.1? → approval1.8.0.1-
Comment 9•19 years ago
|
||
Comment on attachment 199359 [details] [diff] [review] the easy way: throw NS_ERROR_NOT_IMPLEMENTED Renominating for 1.8.0.2 based on the reproducible crash this fixes.
Attachment #199359 -
Flags: branch-1.8.1+
Attachment #199359 -
Flags: approval1.8.1?
Attachment #199359 -
Flags: approval1.8.0.2?
Comment 10•19 years ago
|
||
checked in on MOZILLA_1_8_BRANCH Checking in modules/libpr0n/decoders/icon/nsIconURI.cpp; /cvsroot/mozilla/modules/libpr0n/decoders/icon/nsIconURI.cpp,v <-- nsIconURI.cpp new revision: 1.19.8.1; previous revision: 1.19 done
Updated•19 years ago
|
Keywords: fixed1.8.1
Updated•19 years ago
|
Keywords: fixed1.8.1
Assignee | ||
Comment 11•18 years ago
|
||
Attachment #211984 -
Flags: review?(cbiesinger)
Comment 12•18 years ago
|
||
Comment on attachment 211984 [details] [diff] [review] implement nsMozIconURI::Clone + rv = mFileIcon->Clone (getter_AddRefs (newFileIcon)); please remove the space before the (, that's not the style of this file + nsMozIconURI *uri = new nsMozIconURI(); + if (uri) why not make this an early return as well, like for mFileIcon->Clone? + NS_ADDREF (uri); again a space before ( + uri->mFileIcon = newFileIcon; might want to use .swap here, to avoid a useless addref/release pair
Attachment #211984 -
Flags: review?(cbiesinger) → review+
Comment 13•18 years ago
|
||
Comment on attachment 199359 [details] [diff] [review] the easy way: throw NS_ERROR_NOT_IMPLEMENTED approved for 1.8.0 branch, a=dveditz for drivers
Attachment #199359 -
Flags: approval1.8.0.2? → approval1.8.0.2+
Updated•18 years ago
|
Flags: blocking1.8.0.2+
Keywords: fixed1.8.1
Assignee | ||
Comment 14•18 years ago
|
||
Attachment #211984 -
Attachment is obsolete: true
Attachment #213082 -
Flags: superreview?(bryner)
Attachment #213082 -
Flags: review+
Updated•18 years ago
|
Attachment #213082 -
Flags: superreview?(bryner) → superreview+
Updated•18 years ago
|
Whiteboard: [nvn-dl]
Comment 16•18 years ago
|
||
Did that patch ever get checked in? If not, do you need someone to do that or something?
Flags: blocking1.9a2?
Assignee | ||
Comment 17•18 years ago
|
||
Comment on attachment 213082 [details] [diff] [review] updated patch, addresses reviewer's comments > Did that patch ever get checked in? Not checked in, no, since I have no cvs access and forgot about asking someone else... > If not, do you need someone to do that or something? Yes :)
Updated•18 years ago
|
Assignee: pavlov → chpe
Comment 18•18 years ago
|
||
Fixed on trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Flags: blocking1.9a2?
You need to log in
before you can comment on or make changes to this bug.
Description
•