Remove |weak| parameter from nsIMutableArray methods

RESOLVED FIXED in Firefox 58

Status

()

Core
XPCOM
P3
normal
RESOLVED FIXED
2 years ago
7 months ago

People

(Reporter: erahm, Assigned: emk)

Tracking

(Blocks: 1 bug)

Trunk
mozilla58
Points:
---

Firefox Tracking Flags

(firefox52 wontfix, firefox58 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Currently nsIMutableArray.{appendElement,insertElementAt,replaceElementAt} require a |weak| param. This is rarely, if at all, ever set to |true|. Lets make this param optional and have it default to false.
Mass wontfix for bugs affecting firefox 52.
status-firefox52: affected → wontfix

Updated

7 months ago
Blocks: 1347507
Priority: -- → P3
(Assignee)

Comment 2

7 months ago
Bug 1355216 already fixed about appendElement. But we could just remove the `weak` parameter entirely as Nathan said in that bug.
Summary: nsIMutableArray should default to |weak = false| when adding an element → Remove |weak| parameter from nsIMutableArray methods
Comment hidden (mozreview-request)
(Assignee)

Updated

7 months ago
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED

Comment 4

7 months ago
mozreview-review
Comment on attachment 8920820 [details]
Bug 1313150 - Remove |weak| parameter from nsIMutableArray methods.

https://reviewboard.mozilla.org/r/191808/#review197198

This makes sense to me.  ~150 calls and none of them ask for weak references?  Definitely a candidate for fixing up.

I see that you changed the lone JavaScript test we have for this...how confident are we that there are no other calls from JS that we have to fixup?
Attachment #8920820 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 5

7 months ago
mozreview-review-reply
Comment on attachment 8920820 [details]
Bug 1313150 - Remove |weak| parameter from nsIMutableArray methods.

https://reviewboard.mozilla.org/r/191808/#review197198

I searched insertElementAt and replaceElementAt using dxr and replaced them.
This patch contains 4 js files.

Comment 6

7 months ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 7a68de4eca81 -d e1e574000af4: rebasing 428870:7a68de4eca81 "Bug 1313150 - Remove |weak| parameter from nsIMutableArray methods. r=froydnj" (tip)
merging dom/base/nsDOMWindowUtils.cpp
merging security/manager/ssl/PKCS11ModuleDB.cpp
merging security/manager/ssl/nsNSSCertificateDB.cpp
merging security/manager/ssl/nsNSSIOLayer.cpp
merging security/manager/ssl/nsPKCS11Slot.cpp
merging xpfe/components/directory/nsDirectoryViewer.cpp
warning: conflicts while merging security/manager/ssl/nsPKCS11Slot.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)

Comment 8

7 months ago
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/26c7003f1546
Remove |weak| parameter from nsIMutableArray methods. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/26c7003f1546
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Comment 10

7 months ago
And tons of uses in c-c (e.g. calls to appendElement seem to be all with false). Should be manageable for us.
You need to log in before you can comment on or make changes to this bug.