Found 23 matching lines in 23 files

These are the last actual uses of strres.js in mozilla-central...

(In reply to Cykesiopka from comment #2) > Created attachment 720398 [details] [diff] [review] > Proposed Patch Probably should have split this up into the .xul and .js parts, but oh well...

Comment on attachment 720398 [details] [diff] [review] Proposed Patch David, can you review this? You probably know more about front-end stuff than I do.

Comment on attachment 720398 [details] [diff] [review] Proposed Patch Review of attachment 720398 [details] [diff] [review]: ----------------------------------------------------------------- The usage of stringbundle looks consistent to other uses I've seen. I just have a few comments on formatting, etc. ::: security/manager/pki/resources/content/certManager.js @@ +418,5 @@ > return; > > var params = Components.classes[nsDialogParamBlock].createInstance(nsIDialogParamBlock); > > + var bundle = document.getElementById("pippki_bundle"); It doesn't look like this is even used in this function anymore - you can probably just delete it. ::: security/manager/pki/resources/content/certManager.xul @@ +21,5 @@ > buttons="accept" > style="width: 48em; height: 32em;" > persist="screenX screenY width height"> > > + <stringbundle id="pippki_bundle" src="chrome://pippki/locale/pippki.properties"/> It looks like this is called pkiBundle elsewhere (e.g. page info), but I'm not sure it's that important. ::: security/manager/pki/resources/content/device_manager.js @@ +107,5 @@ > { > var fipsButton = document.getElementById("fipsbutton"); > var label; > if (secmoddb.isFIPSEnabled) { > + label = bundle.getString("disable_fips"); Trailing space here and on the next 15 changed lines. @@ +343,5 @@ > selected_token.login(false); > var tok_status = document.getElementById("tok_status"); > if (selected_token.isLoggedIn()) { > tok_status.setAttribute("label", > + bundle.getString("devinfo_stat_loggedin")); I assume this line and the next changed one have strange alignment due to an 80 character limit. I'm not sure what the correct style is here - maybe Brian will know. @@ +473,5 @@ > function showTokenInfo() > { > //ClearInfoList(); > var selected_token = selected_slot.getToken(); > + AddInfoRow(bundle.getString("devinfo_label"), Trailing space here and again a few lines later. ::: security/manager/pki/resources/content/exceptionDialog.js @@ +55,1 @@ > I'm not sure this empty line was ever necessary here. ::: security/manager/pki/resources/content/pippki.js @@ +152,5 @@ > } > if (written != content.length) { > if (!msg.length) > + msg = bundle.getString("writeFileUnknownError"); > + alertPromptService(bundle.getString("writeFileFailure"), This should be indented to line up with msg (use spaces, please). ::: security/manager/pki/resources/content/pref-crlupdate.js @@ +171,5 @@ > if( cnt > 0 ){ > errorCountText.setAttribute("value",cnt); > errorDetailsText.setAttribute("value",txt); > } else { > + errorCountText.setAttribute("value",bundle.getString("NoUpdateFailure")); Have a space here after the comma.

(In reply to David Keeler (:keeler) from comment #5) > Comment on attachment 720398 [details] [diff] [review] > Proposed Patch > > Review of attachment 720398 [details] [diff] [review]: > ----------------------------------------------------------------- > > The usage of stringbundle looks consistent to other uses I've seen. > I just have a few comments on formatting, etc. > > ::: security/manager/pki/resources/content/certManager.js > @@ +418,5 @@ > > return; > > > > var params = Components.classes[nsDialogParamBlock].createInstance(nsIDialogParamBlock); > > > > + var bundle = document.getElementById("pippki_bundle"); > > It doesn't look like this is even used in this function anymore - you can > probably just delete it. Done. > ::: security/manager/pki/resources/content/certManager.xul > @@ +21,5 @@ > > buttons="accept" > > style="width: 48em; height: 32em;" > > persist="screenX screenY width height"> > > > > + <stringbundle id="pippki_bundle" src="chrome://pippki/locale/pippki.properties"/> > > It looks like this is called pkiBundle elsewhere (e.g. page info), but I'm > not sure it's that important. Mmm, "pippki_bundle" was already in some of the xul files, so I just reused it... I'm leaving it for now, but I can change it to "pkiBundle" later if needed. > ::: security/manager/pki/resources/content/device_manager.js > @@ +107,5 @@ > > { > > var fipsButton = document.getElementById("fipsbutton"); > > var label; > > if (secmoddb.isFIPSEnabled) { > > + label = bundle.getString("disable_fips"); > > Trailing space here and on the next 15 changed lines. Fixed. Hopefully... > @@ +343,5 @@ > > selected_token.login(false); > > var tok_status = document.getElementById("tok_status"); > > if (selected_token.isLoggedIn()) { > > tok_status.setAttribute("label", > > + bundle.getString("devinfo_stat_loggedin")); > > I assume this line and the next changed one have strange alignment due to an > 80 character limit. I'm not sure what the correct style is here - maybe > Brian will know. I went ahead and aligned them. The rest of the code has mostly aligned stuff, and it doesn't seem like this line or the others like it break 80 chars post alignment anyways... > @@ +473,5 @@ > > function showTokenInfo() > > { > > //ClearInfoList(); > > var selected_token = selected_slot.getToken(); > > + AddInfoRow(bundle.getString("devinfo_label"), > > Trailing space here and again a few lines later. Again hopefully fixed. > ::: security/manager/pki/resources/content/exceptionDialog.js > @@ +55,1 @@ > > > > I'm not sure this empty line was ever necessary here. Done. > ::: security/manager/pki/resources/content/pippki.js > @@ +152,5 @@ > > } > > if (written != content.length) { > > if (!msg.length) > > + msg = bundle.getString("writeFileUnknownError"); > > + alertPromptService(bundle.getString("writeFileFailure"), > > This should be indented to line up with msg (use spaces, please). Hmm, aren't they of different logic levels...? The if statement isn't braced... > ::: security/manager/pki/resources/content/pref-crlupdate.js > @@ +171,5 @@ > > if( cnt > 0 ){ > > errorCountText.setAttribute("value",cnt); > > errorDetailsText.setAttribute("value",txt); > > } else { > > + errorCountText.setAttribute("value",bundle.getString("NoUpdateFailure")); > > Have a space here after the comma. Done. Thanks for the review!

Needs to be reviewed by a module peer first.

Comment on attachment 721123 [details] [diff] [review] Patch for check in Review of attachment 721123 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch. I will review this in the next couple of days.

(In reply to Brian Smith (:bsmith) from comment #8) > Needs to be reviewed by a module peer first. Oops! Sorry about that...

LGTM. David, when you reviewed this, did you check out the results of the build? There are no tests for this stuff so it's worth making sure that nothing broke before checking it in. I can test it if you didn't.

(In reply to Brian Smith (:bsmith) from comment #11) > LGTM. > > David, when you reviewed this, did you check out the results of the build? > There are no tests for this stuff so it's worth making sure that nothing > broke before checking it in. I can test it if you didn't. I didn't actually test it out, no. (In reply to Cykesiopka from comment #6) > > ::: security/manager/pki/resources/content/pippki.js > > @@ +152,5 @@ > > > } > > > if (written != content.length) { > > > if (!msg.length) > > > + msg = bundle.getString("writeFileUnknownError"); > > > + alertPromptService(bundle.getString("writeFileFailure"), > > > > This should be indented to line up with msg (use spaces, please). > > Hmm, aren't they of different logic levels...? The if statement isn't > braced... Ah, yes - good catch :)

bsmith: can this be checked in now, or is testing still needed?

David, could you please test this out and check it in? Otherwise, I can do so, but there will be a pretty big delay.

I built locally with no problems, tried out some of the UI that would have been affected, and everything looked good. Here's a try run for what it's worth: https://tbpl.mozilla.org/?tree=Try&rev=b36f7e42b117 I'll check that in if everything looks good.

V.Fixed, per MXR search. (Thanks!)