search

infinity0 / mozilla-gnome-keyring-legacy

A firefox extension that enables Gnome Keyring integration (legacy version)
https://bugzilla.mozilla.org/show_bug.cgi?id=309807
Other
55 stars 8 forks source link

Firefox v32.0: cannot compile #46

Closed AndreasA closed 9 years ago

AndreasA commented 9 years ago

Hi,

I cannot compile it against the v32.0 SDK, I get the following output:

../SDKs/xulrunner/v32.0/xulrunner-sdk/include/jspubtd.h:512:51: warning: invalid access to non-static data member ‘js::PerThreadDataFriendFields::RuntimeDummy::mainThread’  of NULL object [-Winvalid-offsetof]
../SDKs/xulrunner/v32.0/xulrunner-sdk/include/jspubtd.h:512:51: warning: (perhaps the ‘offsetof’ macro was used incorrectly) [-Winvalid-offsetof]
GnomeKeyring.cpp:582:34: error: no ‘nsresult GnomeKeyring::Init()’ member function declared in class ‘GnomeKeyring’
GnomeKeyring.cpp:631:61: error: no ‘nsresult GnomeKeyring::InitWithFile(nsIFile*)’ member function declared in class ‘GnomeKeyring’
GnomeKeyring.cpp:770:66: error: no ‘nsresult GnomeKeyring::GetAllEncryptedLogins(unsigned int*, nsILoginInfo***)’ member function declared in class ‘GnomeKeyring’
GnomeKeyring.cpp:86:20: warning: ‘kPrefsBranch’ defined but not used [-Wunused-variable]
GnomeKeyring.cpp:87:20: warning: ‘kPrefsKeyring’ defined but not used [-Wunused-variable]
GnomeKeyring.cpp:88:20: warning: ‘kDefaultKeyring’ defined but not used [-Wunused-variable]
AndreasA commented 9 years ago

It seems like they modified the nsILoginManagerStorage.h the initWithFile method seems to have been removed completely and the Init Method looks now like this: NS_IMETHOD Initialize(JS::MutableHandleValue _retval) = 0;

GetAllEncryptedLogins seems also to have been removed

AndreasA commented 9 years ago

I have just created a pull request which fixes the this problem: https://github.com/infinity0/mozilla-gnome-keyring/pull/47

I tried to compile the fix with v31.0 and v32.0 and both work correctly (also installed and checked if I can access the passwords).

What I have not added is support for the new last used / stored timestamps.

infinity0 commented 9 years ago

Thanks! I will try to review and merge this in the next week.

infinity0 commented 9 years ago

Should be fixed. I simplified the code to remove support for older versions.

I'll note now that our Initialize isn't conformant to the new IDL; we are supposed to return a "Promise" JS object, but I don't know how to do that from C++. In practise I don't think it is a problem though, but eventually we should move to swick's JS implementation (#43) when that is ready.

AndreasA commented 9 years ago

I think that is in general a good idea. It should not require to be recompiled for every version that way, shouldn't it?

infinity0 commented 9 years ago

Recompiled no, but a new version might still break it. For example the Init -> Initialize change here, requires changes in JS too, e.g. see swick/moz-gnome-keyring-integration@f0423833b839a4ffcbf442f5822a023e05861cef However more trivial changes such as the PRUnichar -> char16_t around version 29/30 (can't remember which) should not affect JS.