password manager tests rely on deprecated XPConnect behavior

RESOLVED FIXED in mozilla21

Status

()

Toolkit
Password Manager
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla21
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
In toolkit/components/passwordmgr/test/unit/test_storage_mozStorage_6.js, test 11 creates a JS-implemented "wonkyLogin" object and monkeypatches it with XPCOM methods from "wonkyDelegate", a bonafide nsLoginInfo object. This assumes that the C++ methods know how to find the object on which they were defined even if invoked with a different |this| object.

I got rid of this behavior in quickstubs in january (bug 622301), and I'm finally getting rid of it for old-fashioned XPConnect calls in bug 658909.

The tests seem to pass with this check removed, so hopefully reviewers can tell me if there's any real password manager code that actually relies on this being possible.
(Assignee)

Comment 1

6 years ago
Created attachment 689851 [details] [diff] [review]
Stop relying on deprecated behavior in passwordmgr mozStorage tests. v1
Attachment #689851 - Flags: review?(dolske)
Comment on attachment 689851 [details] [diff] [review]
Stop relying on deprecated behavior in passwordmgr mozStorage tests. v1

Review of attachment 689851 [details] [diff] [review]:
-----------------------------------------------------------------

IIRC from bug 477917, the general purpose here was to try and not break any theoretical implementers of nsILoginInfo who didn't also implement nsILoginMetaInfo. And so I added this test with such an implementation to make sure it kept working.

I don't think that's actually useful to support (nor does, afaik, anyone actually do this). So let's just remove this (as you've done) and not look for an alternate way of testing.
Attachment #689851 - Flags: review?(dolske) → review+
https://hg.mozilla.org/mozilla-central/rev/07d17a3ff4f9
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.