653775 - Investigate Springpad compatibility with Firefox 5

Status: RESOLVED WONTFIX

(Firefox :: General, defect, P1)

Description

[Justin Scott [:fligtar]]

While testing in Aurora 5, I wasn't able to log in to the Springpad add-on. After troubleshooting a bit, I tried in Firefox 4 and it worked fine.

Please investigate to see if this is this is an incompatibility in the SDK.

https://addons.mozilla.org/en-US/firefox/addon/springpad/

You can use my test account if you'd like:
user: fligtar
pass: test

Comment 1

[Justin Scott [:fligtar]]

this error appears in the error console when the login fails:

Error: [Exception... "'PopupNotifications_show: invalid browser' when calling method: [nsILoginManagerPrompter::promptToSavePassword]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  location: "JS frame :: resource://gre/components/nsLoginManager.js :: <TOP_LEVEL> :: line 959"  data: no]
Source File: resource://gre/components/nsLoginManager.js
Line: 959

Comment 2

[Jorge Villalobos [:jorgev] (he/him)]

Looking at the network activity during login, I noticed that in Aurora, the get-token call is sent using the OPTIONS method, which is extremely odd.

Firefox 4:
13:55:31.725[610ms][total 610ms] Status: 200[OK]
GET https://springpadit.com/api/users/get-token Load Flags[LOAD_BACKGROUND  ] Content Size[227] Mime Type[application/javascript]

Aurora:
13:56:12.824[203ms][total 203ms] Status: 200[OK]
OPTIONS https://springpadit.com/api/users/get-token Load Flags[LOAD_BACKGROUND  ] Content Size[20] Mime Type[text/plain]

Digging into the code, the add-on uses a regular XMLHttpRequest object, nothing particularly related to the SDK.

I'm not sure if this is related to the error in the Error Console.

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Relevant stuff from that part of nsLoginManager.js:

// Prompt user to save login (via dialog or notification bar)
prompter = getPrompter(win);
prompter.promptToSavePassword(formLogin);

I'd guess that it can't get to the window element 'win', since it's in the panel using the SDK's panel black magic. (Or it can get to 'win' but there's no prompter for it.)

getPrompter does this:
// local helper function
function getPrompter(aWindow) {
    var prompterSvc = Cc["@mozilla.org/login-manager/prompter;1"].
                      createInstance(Ci.nsILoginManagerPrompter);
    prompterSvc.init(aWindow);
    return prompterSvc;
}



Turning on signon.debug in about:config shows the following errors/warnings when opening the panel and trying to log in:
http://pastebin.mozilla.org/1215550

Comment 4

[Justin Scott [:fligtar]]

Any news on this? If something changed here between 4 and 5 we need to know ASAP whether specific to the SDK or not.

Comment 5

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

Alex: can you take a look at this?

Comment 6

[Alexandre Poirot [:ochameau]]

(In reply to comment #3)
You are right, there is an exception here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/PopupNotifications.jsm#236
From Popup notifications because we are not able to retrieve the `browser` reference here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#870
As we are retrieving the panel window reference instead of a tab window here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/nsLoginManager.js#832

Having said that, I looked at some blame on these files and I was not able to find any change made for Aurora.
But I'm pretty sure that a modification has been made in platform code to change this behavior. These notifications may appear on more case in Aurora than in Firefox 4.0.
I need more thought on this topic and may be some advice from platform team.

But you may solve this problem today, using this kind of code:
  var loginmgr = Cc["@mozilla.org/login-manager;1"].createInstance(Ci.nsILoginManager);
  loginmgr.setLoginSavingEnabled("http://springboard.com", false);
That will disable the code that is failing now.

Comment 7

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

(In reply to comment #4)
> Any news on this? If something changed here between 4 and 5 we need to know
> ASAP whether specific to the SDK or not.

The problem probably isn't specific to the SDK, but it may be primarily triggered by SDK-based addons, since traditional addons typically use XUL to build panels, which probably wouldn't trigger the problem, whereas the SDK's Panel API solicits HTML from addons and loads it in an iframe.

There may be a workaround on the SDK side of things, but this sounds like a regression in Firefox 5.

Comment 8

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

By the way, Springboard throws the following error and fails to add its toolbar button to the navigation toolbar if you install it when the Error Console is open:

error: An exception occurred.
Traceback (most recent call last):
  File "resource://ext-at-sprng-dot-me-springpad-lib/main.js", line 187, in 
    addToolbarIcon(window.document);
  File "resource://ext-at-sprng-dot-me-springpad-lib/main.js", line 22, in addToolbarIcon
    toolbar.appendChild(button);
TypeError: toolbar is null

That's because it iterates windows and tries to add its toolbar button to the navigation toolbar of each one, but Error Console doesn't have that toolbar.

The fix is to change line 20 of main.js:

-  if(!doc.querySelector('.springpad-extension')){
+  if(toolbar && !doc.querySelector('.springpad-extension')){

Comment 9

[Justin Dolske [:Dolske]]

Hmm, I'm not sure we ever explicitly tested/supported the password manager for content loading in places other than normal tabs. But I think it _should_ be falling back to the old-style modal prompts (because we don't have a URL bar or tab for the doorhanger/notificationbar to attach to).

What did password manager do in FF4 with this addon?

Comment 10

[Justin Dolske [:Dolske]]

Or perhaps password manager simply shouldn't handle content documents outside of tabs, and only provide hooks for addons to trigger password manager [ala existing fillDoc()] or flag the iframe as being appropriate for password management. Hmm.

Comment 11

[Alexandre Poirot [:ochameau]]

By any chance, do you know if any such flags already exists ?
We may fix this bug and then work on implementing this feature back to normal.

Comment 12

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

Based on the analysis so far, this appears to be a regression in Firefox 5 that needs a Firefox fix to resolve, so moving it to the Firefox product.

Justin: can you take this on?

Comment 13

[:Gavin Sharp [email: [e-mail]]]

This addon seems to no longer exist.