Comment on attachment 706354
Store Master Password to Gnome Keyring patch v1

You need build system and PSM reviews, so I'll just comment on code
patterns.

>+      PRUnichar *password = nullptr;
>+      nsAutoString promptString;
>+      
>+      // Get prompt message for Gnome Keyring master password
>+      nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, 
>&rv));
>+      if (NS_FAILED(rv))
>+        return;
>+
>+      const PRUnichar* formatStrings[1] = { 
>+        ToNewUnicode(NS_LITERAL_STRING("Gnome Keyring"))
>+      };
NS_NAMED_LITERAL_STRING gnomeKeyring("Gnome Keyring");
const PRUnichar* formatStrings[] = { gnomeKeyring.get() };
Or since this is linux-only, MOZ_LL, NS_LL or u"Gnome Keyring" might work.
This avoids having to free the string manually.

>+        // Return if promp was closed without confirmation or password is 
>wrong
Typo: prompt

>+        if (value)
[I had to look up the .idl to find out what this value meant.]

>+          nsCString utf8password;
>+          utf8password = NS_ConvertUTF16toUTF8(password);
>+          NS_Free(password);
>+          password = nullptr;
>+          if (gnome_keyring_unlock_sync(NULL, utf8password.get()) == 
>GNOME_KEYRING_RESULT_OK)
gnome_keyring_unlock_sync(NULL, NS_ConvertUTF16toUTF8(password).get())

>+        mResult = g_strdup(keyringRecord->secret);
[Does this get freed with g_free?]

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to firefox in Ubuntu.
https://bugs.launchpad.net/bugs/41179

Title:
  Integrate with Gnome Keyring

Status in The Mozilla Firefox Browser:
  Confirmed
Status in “firefox” package in Ubuntu:
  Won't Fix
Status in “xulrunner-1.9” package in Ubuntu:
  Won't Fix
Status in “xulrunner-1.9.1” package in Ubuntu:
  Triaged

Bug description:
  For a really good Gnome integration, it would be great to have the
  ability to save passwords in the Gnome keyring.

  A similar thing has been proposed for Epiphany: see
  https://launchpad.net/malone/bugs/3467.

To manage notifications about this bug go to:
https://bugs.launchpad.net/firefox/+bug/41179/+subscriptions

-- 
Mailing list: https://launchpad.net/~desktop-packages
Post to     : desktop-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~desktop-packages
More help   : https://help.launchpad.net/ListHelp

Reply via email to