(In reply to comment #66)
> >+ // Don't trigger a 2nd master password prompt (it's
> >+ // unlikely we'll get here, because we normally don't
> >+ // attach this listener until after a MP entry).
> So how would we get to this case?
Hmm, I guess we can't. I was thinking some mix of loading a login page, canceling the resulting MP prompt, and then later revisiting the page to type in a login... But the MP cancelation would throw, and fillDoc() would thus bail out before attaching an autocomplete listener. So, not just edge-casy but shouldn't be possible. Removed. Ditto for the other case of this.
> > if (!this._remember)
> >- return false;
> >+ return null;
> What's this change for?
Noticed while checking code that autoCompleteSearch is supposed to return an nsIAutoCompleteResult (or null), not a bool.
> >+ doc.addEventListener("mozCleverClosureHack", observer, false);
> Trickyness indeed. Is this something we do elsewhere in the code?
Yeah, I first used it over in browser.js for some plugin stuff.
> Didn't we already do the hasLogins check when adding the prompt? Isn't it just
> |prompt.hasLogins| & we can avoid this additional DB hit?
Hmm, good catch. I think that's deadwood from an early version of the patch. I've removed the |prompt.hasLogins| setting, since it wasn't used. Better to check it right when needed (otherwise we could miss a recently-saved login).