Comment 29 for bug 217300

Revision history for this message
In , Sylvain Pasche (sylvain-pasche) wrote :

(In reply to comment #19)
> There are some more things that could be improved about the code, but what are
> the steps to get this included in Firefox proper?

Hi Matt, thanks for your work on this. The steps would be to find someone to review this. There are instructions on http://developer.mozilla.org/en/Getting_your_patch_in_the_tree how to find the right reviewer.

Another more general question is to know if this should be part of Firefox or should live as an extension.

On a more technical side, is the issue raised in comment 4 still there? That would be nice to have it addressed (it may not be trivial, if the API has to change to get asynchronous).

I was a bit worried about performance (comment 6). It could be interesting to see how having the addon enabled can impact performance when browsing pages with passwords (https://wiki.mozilla.org/StandaloneTalos is one of the tool for measuring regressions). Caching could help if the performance is an issue.

The current patch implements nsILoginManagerStorage directly in C++. Another approach I thought about would be to wrap the low-level Gnome Keyring API in a C++ component and then implement nsILoginManagerStorage in JS. That should make things simpler for adding caching (if needed), managing preferences (keyring name is hardcoded right now for instance) or others. (https://wiki.mozilla.org/JSctypes could be useful here but I don't think it's ready yet)