Last Comment Bug 626626 - Password manager references removed strings (saveLoginText, saveLoginTextNoUsername)
: Password manager references removed strings (saveLoginText, saveLoginTextNoUs...
Status: RESOLVED FIXED
: regression
Product: Toolkit
Classification: Components
Component: Password Manager (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla9
Assigned To: Felix Fung (:felix)
:
: Matthew N. [:MattN] (PM me if requests are blocking you)
Mentors:
Depends on:
Blocks: 594572
  Show dependency treegraph
 
Reported: 2011-01-18 06:00 PST by Mark Banner (:standard8, afk until Dec)
Modified: 2011-09-14 12:17 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Reference Correct Strings in Password Manager (2.26 KB, patch)
2011-09-07 16:26 PDT, Felix Fung (:felix)
dolske: review-
Details | Diff | Splinter Review
Reference Correct Strings in Password Manager (4.34 KB, patch)
2011-09-08 12:38 PDT, Felix Fung (:felix)
dolske: review+
Details | Diff | Splinter Review

Description Mark Banner (:standard8, afk until Dec) 2011-01-18 06:00:21 PST
I was just looking at some l10n stuff for Thunderbird and noticed the following in mozilla-central:

http://hg.mozilla.org/mozilla-central/annotate/acb48b922e91/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js#l970

        if (aLogin.username) {
            var displayUser = this._sanitizeUsername(aLogin.username);
            dialogText = this._getLocalizedString(
                                 "saveLoginText",
                                 [brandShortName, displayUser, displayHost]);
        } else {
            dialogText = this._getLocalizedString(
                                 "saveLoginTextNoUsername",
                                 [brandShortName, displayHost]);
        }
Both those strings are no longer defined in l10n land. I'm not sure if this code will really be hit (probably not frequently) but I suspect it isn't going to give good results if it does.

This is a regression from bug 594572.
Comment 1 Justin Dolske [:Dolske] 2011-01-18 13:15:19 PST
Fuuuuuu... :/

Oh, but we can't actually hit this in Firefox-land because this code only gets invoked if it can't figure out how to get at a doorhanger or notification bar. [Well, ok, we probably could if an addond created a non-<browser> thing in which a user could browser and submit forms, but that's rather edge-caseish.]
Comment 2 Felix Fung (:felix) 2011-09-07 16:26:56 PDT
Created attachment 558993 [details] [diff] [review]
Reference Correct Strings in Password Manager

Restored the changes produced by the patch to https://bugzilla.mozilla.org/show_bug.cgi?id=594572
Comment 3 Justin Dolske [:Dolske] 2011-09-07 19:27:26 PDT
Comment on attachment 558993 [details] [diff] [review]
Reference Correct Strings in Password Manager

Review of attachment 558993 [details] [diff] [review]:
-----------------------------------------------------------------

I went through the other strings changed in bug 594572...

2 other strings -- in _showChangeLoginDialog() -- are also broken. (passwordChangeText, passwordChangeTextNoUser).

Let's go ahead and fix those too.

::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js
@@ -956,5 @@
>              (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_1) +
>              (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_2);
>  
> -        var brandShortName =
> -                this._brandBundle.GetStringFromName("brandShortName");

This was the only use of ._brandBundle, so go ahead and delete that getter (line ~268).
Comment 4 Felix Fung (:felix) 2011-09-08 12:38:50 PDT
Created attachment 559250 [details] [diff] [review]
Reference Correct Strings in Password Manager

Addressed above comments
Comment 6 :Margaret Leibovic 2011-09-14 12:17:43 PDT
https://hg.mozilla.org/mozilla-central/rev/0a36d58f2b4f

Note You need to log in before you can comment on or make changes to this bug.