Closed
Bug 705100
Opened 12 years ago
Closed 12 years ago
remove nsISupportsArray from migration
Categories
(Firefox :: Migration, defect)
Firefox
Migration
Tracking
()
RESOLVED
FIXED
Firefox 11
People
(Reporter: MattN, Assigned: MattN)
Details
Attachments
(1 file, 2 obsolete files)
14.30 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
nsISupportsArray was obsoleted by nsIArray and nsIMutableArray. Replace the usage in browser/components/migration as they provide a more natural interface for JS.
Attachment #576717 -
Flags: review?(mak77)
Comment 1•12 years ago
|
||
Comment on attachment 576717 [details] [diff] [review] Replace nsISupportsArray with nsIArray and nsIMutableArray in migration Review of attachment 576717 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/migration/src/nsOperaProfileMigrator.cpp @@ +209,5 @@ > if (!mProfiles) { > nsresult rv; > > mProfiles = do_CreateInstance(NS_SUPPORTSARRAY_CONTRACTID, &rv); > if (NS_FAILED(rv)) return rv; This looks wrong, shouldn't use NS_ARRAY_CONTRACTID?
Comment 2•12 years ago
|
||
Comment on attachment 576717 [details] [diff] [review] Replace nsISupportsArray with nsIArray and nsIMutableArray in migration clearing while the above gets clarified
Attachment #576717 -
Flags: review?(mak77)
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #1) > Review of attachment 576717 [details] [diff] [review] [diff] [details] [review]: > ----------------------------------------------------------------- > > ::: browser/components/migration/src/nsOperaProfileMigrator.cpp > @@ +209,5 @@ > > if (!mProfiles) { > > nsresult rv; > > > > mProfiles = do_CreateInstance(NS_SUPPORTSARRAY_CONTRACTID, &rv); > > if (NS_FAILED(rv)) return rv; > > This looks wrong, shouldn't use NS_ARRAY_CONTRACTID? You're right, I refreshed that into the wrong patch. Here is a fixed version.
Assignee: nobody → mnoorenberghe+bmo
Attachment #576717 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #577108 -
Flags: review?(mak77)
Assignee | ||
Comment 4•12 years ago
|
||
Not sure how I missed this
Attachment #577108 -
Attachment is obsolete: true
Attachment #577108 -
Flags: review?(mak77)
Attachment #577502 -
Flags: review?(mak77)
Comment 5•12 years ago
|
||
Comment on attachment 577502 [details] [diff] [review] v0.3 Another missing change Review of attachment 577502 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/migration/content/migration.js @@ +215,2 @@ > for (var i = 0; i < count; ++i) { > var item = document.createElement("radio"); I don't see the point of the count variable here, couldn't you inline .length in the for condition? ::: browser/components/migration/src/nsProfileMigrator.cpp @@ +127,5 @@ > // By opening the Migration FE with a supplied bpm, it will automatically > // migrate from it. > nsCOMPtr<nsIWindowWatcher> ww(do_GetService(NS_WINDOWWATCHER_CONTRACTID)); > + nsCOMPtr<nsIMutableArray> params = > + do_CreateInstance(NS_ARRAY_CONTRACTID); it's possible you can oneline this, the old impl was going over 80 chars, this looks fine instead
Attachment #577502 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b6d9e92b148
Flags: in-testsuite-
Target Milestone: --- → Firefox 11
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3b6d9e92b148
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•