Last Comment Bug 312241 - nsIconURI::Clone and ::Resolve are unimplemented
: nsIconURI::Clone and ::Resolve are unimplemented
Status: RESOLVED FIXED
[nvn-dl]
: fixed1.8.0.2, fixed1.8.1
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: x86 Linux
: -- critical (vote)
: ---
Assigned To: Christian Persch (GNOME) (away; not receiving bug mail)
:
Mentors:
Depends on:
Blocks: 320621
  Show dependency treegraph
 
Reported: 2005-10-12 15:44 PDT by Christian Persch (GNOME) (away; not receiving bug mail)
Modified: 2006-11-10 12:32 PST (History)
6 users (show)
dveditz: blocking1.8.0.2+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
the easy way: throw NS_ERROR_NOT_IMPLEMENTED (1.06 KB, patch)
2005-10-12 15:50 PDT, Christian Persch (GNOME) (away; not receiving bug mail)
cbiesinger: review+
bryner: superreview+
benjamin: approval‑branch‑1.8.1+
mtschrep: approval1.8.0.1-
dveditz: approval1.8.0.2+
Details | Diff | Splinter Review
implement nsMozIconURI::Clone (2.80 KB, patch)
2006-02-15 06:20 PST, Christian Persch (GNOME) (away; not receiving bug mail)
cbiesinger: review+
Details | Diff | Splinter Review
updated patch, addresses reviewer's comments (2.58 KB, patch)
2006-02-24 12:51 PST, Christian Persch (GNOME) (away; not receiving bug mail)
chpe: review+
bryner: superreview+
Details | Diff | Splinter Review

Description Christian Persch (GNOME) (away; not receiving bug mail) 2005-10-12 15:44:37 PDT
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.
Comment 1 Christian Persch (GNOME) (away; not receiving bug mail) 2005-10-12 15:50:06 PDT
Created attachment 199359 [details] [diff] [review]
the easy way: throw NS_ERROR_NOT_IMPLEMENTED
Comment 2 Christian :Biesinger (don't email me, ping me on IRC) 2005-11-08 12:13:54 PST
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 Steve England [:stevee] 2005-12-13 05:56:55 PST
Can this bug now be RESOLVED FIXED?
Comment 4 Christian Persch (GNOME) (away; not receiving bug mail) 2005-12-13 06:11:24 PST
(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. 
Comment 5 Steve England [:stevee] 2005-12-13 06:17:44 PST
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 Steve England [:stevee] 2005-12-13 06:18:25 PST
Or maybe i'm totally wrong and should shut up :)
Comment 7 Reed Loden [:reed] (use needinfo?) 2005-12-17 12:02:26 PST
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.
Comment 8 Mike Schroepfer 2005-12-19 15:56:09 PST
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.
Comment 9 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2006-01-30 12:21:36 PST
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.
Comment 10 Christian :Biesinger (don't email me, ping me on IRC) 2006-02-01 08:53:06 PST
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
Comment 11 Christian Persch (GNOME) (away; not receiving bug mail) 2006-02-15 06:20:42 PST
Created attachment 211984 [details] [diff] [review]
implement nsMozIconURI::Clone
Comment 12 Christian :Biesinger (don't email me, ping me on IRC) 2006-02-17 12:42:10 PST
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
Comment 13 Daniel Veditz [:dveditz] 2006-02-22 12:39:28 PST
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
Comment 14 Christian Persch (GNOME) (away; not receiving bug mail) 2006-02-24 12:51:29 PST
Created attachment 213082 [details] [diff] [review]
updated patch, addresses reviewer's comments
Comment 15 Daniel Veditz [:dveditz] 2006-02-26 11:29:33 PST
fix checked into 1.8.0 branch
Comment 16 Boris Zbarsky [:bz] 2006-04-21 21:23:23 PDT
Did that patch ever get checked in?  If not, do you need someone to do that or something?
Comment 17 Christian Persch (GNOME) (away; not receiving bug mail) 2006-04-22 07:05:35 PDT
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 :)
Comment 18 Boris Zbarsky [:bz] 2006-04-23 12:02:34 PDT
Fixed on trunk.

Note You need to log in before you can comment on or make changes to this bug.