Closed
Bug 610607
Opened 14 years ago
Closed 13 years ago
suggested Password Manager API implementation
Categories
(Add-on SDK Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mcepl, Assigned: irakli)
References
()
Details
Attachments
(2 files)
I have mistakenly put this into bug 568937, but still I believe it could be worthy not to have blank space next to Password Manager on https://wiki.mozilla.org/Labs/Jetpack/SDK/APIs
Comment 1•14 years ago
|
||
Awesome, thanks for posting this! Next step would be to put up some code examples and send to the mailing list for API feedback. I added this to the SDK APIs page. While we all agreed this is key for 1.0, it's not yet slotted into a release AFAICT.
Comment 2•14 years ago
|
||
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product. To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
Version: Trunk → unspecified
Assignee | ||
Comment 4•13 years ago
|
||
Pasting Myk's comment here (In reply to comment #4) > Comment on attachment 515652 [details] > Documentation that serves as an API proposal. > > This looks great. The API is explicitly asynchronous, which is different from > our other high-level APIs, but it accesses data stored in an SQLite database, > which can potentially take a long time, so the asynchronicity makes sense, and > it is consistent with the proposed Places API (which also accesses SQLite data) > in bug 545700. > > There are a few minor naming issues: > > * The module should be called `passwords`, which is the simplest obvious name > for it, is the most accurate name (as the API exposes credentials that may or > may not include usernames but always include passwords), is consistent with the > naming and description of the feature in the Firefox interface, and is > consistent with the naming of other high-level APIs (although, unlike those > other APIs, its exports object is not iterable, a potential downside of this > name). > * `onDone` should be called `onComplete`, which is consistent with the Request > and proposed Places APIs and is also more common in other JavaScript libraries. > * `user` should be called `username` (and `userField` `usernameField`), which > is a more common name for such data. > * `find` should be called `search`, which is consistent with the proposed > Places API. > > Finally, note that the three primary use cases for this API to support, in > order of priority (from higher to lower), are: > > 1. Retrieve credentials for a website, so the addon can access the user's > account on the website and retrieve information about the user. > 2. Store credentials that are associated with the addon rather than a > particular website, so the addon can access them in subsequent sessions. > 3. Store credentials that are associated with a particular website so both the > addon and the user (when visiting the site without the addon) can access them > in subsequent sessions. > > Thus the documentation should describe the `search` method first and the most > thoroughly. > > A few more notes: > > * If the `url` value includes a path, we should strip it out automatically. > * When an addon stores a password that is not associated with a particular > website, the password should be associated with the addon itself (f.e. by > automatically setting the URL of the password to a chrome: or resource: URL > that uniquely identifies the addon).
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → rFobic
Assignee | ||
Comment 5•13 years ago
|
||
Please note that API review was done under the Bug 637292.
Assignee | ||
Comment 6•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #519145 -
Flags: review?(myk)
Comment 7•13 years ago
|
||
Comment on attachment 519145 [details]
Pointer to pull request
Dietrich: can you take on this review as well?
Attachment #519145 -
Flags: review?(myk) → review?(dietrich)
Comment 8•13 years ago
|
||
Yep. Irakli, are Myk's comments addressed in the latest version?
Assignee | ||
Comment 9•13 years ago
|
||
Yes I believe they are
Assignee | ||
Comment 10•13 years ago
|
||
Dietrich I have updated pull request to address your comments made there.
Comment 11•13 years ago
|
||
Comment on attachment 519145 [details]
Pointer to pull request
r+ during the irc+pull-request review process.
Attachment #519145 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 12•13 years ago
|
||
https://github.com/mozilla/addon-sdk/commit/3dd1747479c7966ff7b9a0399857dcd874d67183
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•