610607 - suggested Password Manager API implementation

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Matěj Cepl]

Attachment: suggested module

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

[Dietrich Ayala (:dietrich)]

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

[Myk Melez [:myk] [@mykmelez]]

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".

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

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).

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Please note that API review was done under the Bug 637292.

Comment 6

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to pull request

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 519145 [details]
Pointer to pull request

Dietrich: can you take on this review as well?

Comment 8

[Dietrich Ayala (:dietrich)]

Yep. Irakli, are Myk's comments addressed in the latest version?

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Yes I believe they are

Comment 10

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Dietrich I have updated pull request to address your comments made there.

Comment 11

[Dietrich Ayala (:dietrich)]

Comment on attachment 519145 [details]
Pointer to pull request

r+ during the irc+pull-request review process.

Comment 12

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

https://github.com/mozilla/addon-sdk/commit/3dd1747479c7966ff7b9a0399857dcd874d67183