Closed Bug 949821 Opened 6 years ago Closed 6 years ago

replace a bunch of NS_LITERAL_STRING uses with MOZ_UTF16

Categories

(Core :: String, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Details

Attachments

(1 file)

No description provided.
generated with sed -i 's/NS_LITERAL_STRING("\(.*\)").get()/MOZ_UTF16("\1")/
Attachment #8346964 - Flags: review?(nfroyd)
Comment on attachment 8346964 [details] [diff] [review]
use MOZ_UTF16 more and NS_LITERAL_STRING less

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

r/rs=me if it builds and passes tests everywhere.
Attachment #8346964 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/0fe105b41eda
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
I landed a trivial fix for mingw:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5eaf6fe48be2

On mingw, wchar_t != char16_t, so we need L"..." instead of MOZ_UTF16 for wchar_t* strings.
Comment on attachment 8346964 [details] [diff] [review]
use MOZ_UTF16 more and NS_LITERAL_STRING less

>diff --git a/security/manager/ssl/src/nsCrypto.cpp b/security/manager/ssl/src/nsCrypto.cpp
>--- a/security/manager/ssl/src/nsCrypto.cpp
>+++ b/security/manager/ssl/src/nsCrypto.cpp
>@@ -2119,20 +2119,20 @@ nsP12Runnable::Run()
> 
>   nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, &rv));
>   if (NS_FAILED(rv))
>     return rv;
> 
>   //Build up the message that let's the user know we're trying to 
>   //make PKCS12 backups of the new certs.
>   nssComponent->GetPIPNSSBundleString("ForcedBackup1", final);
>-  final.Append(NS_LITERAL_STRING("\n\n").get());
>+  final.Append(MOZ_UTF16("\n\n"));
>   nssComponent->GetPIPNSSBundleString("ForcedBackup2", temp);
>   final.Append(temp.get());
>-  final.Append(NS_LITERAL_STRING("\n\n").get());
>+  final.Append(MOZ_UTF16("\n\n"));
Append(NS_LITERAL_STRING(foo).get()) was an awful antipattern. I'm not sure whether Append(NS_LITERAL_STRING(foo)) or AppendLiteral(foo) is better. (Or wait until I get review on bug 514173 and use AppendLiteral(MOZ_UTF16(foo)). But not Append(MOZ_UTF16(foo)) because that still calls NS_strlen.)
(In reply to neil@parkwaycc.co.uk from comment #7)
> Comment on attachment 8346964 [details] [diff] [review]
> use MOZ_UTF16 more and NS_LITERAL_STRING less
> 
> >diff --git a/security/manager/ssl/src/nsCrypto.cpp b/security/manager/ssl/src/nsCrypto.cpp
> >--- a/security/manager/ssl/src/nsCrypto.cpp
> >+++ b/security/manager/ssl/src/nsCrypto.cpp
> >@@ -2119,20 +2119,20 @@ nsP12Runnable::Run()
> > 
> >   nsCOMPtr<nsINSSComponent> nssComponent(do_GetService(kNSSComponentCID, &rv));
> >   if (NS_FAILED(rv))
> >     return rv;
> > 
> >   //Build up the message that let's the user know we're trying to 
> >   //make PKCS12 backups of the new certs.
> >   nssComponent->GetPIPNSSBundleString("ForcedBackup1", final);
> >-  final.Append(NS_LITERAL_STRING("\n\n").get());
> >+  final.Append(MOZ_UTF16("\n\n"));
> >   nssComponent->GetPIPNSSBundleString("ForcedBackup2", temp);
> >   final.Append(temp.get());
> >-  final.Append(NS_LITERAL_STRING("\n\n").get());
> >+  final.Append(MOZ_UTF16("\n\n"));
> Append(NS_LITERAL_STRING(foo).get()) was an awful antipattern. I'm not sure
> whether Append(NS_LITERAL_STRING(foo)) or AppendLiteral(foo) is better. (Or
> wait until I get review on bug 514173 and use AppendLiteral(MOZ_UTF16(foo)).
> But not Append(MOZ_UTF16(foo)) because that still calls NS_strlen.)

While I'm not opposed to improving usage of strings it hardly seems like an autogenerated patch to get rid of one bit of sillyness is the place to go through looking for things to optimize.  So just file a bug to fix that case?
(In reply to neil@parkwaycc.co.uk from comment #7)
> Append(NS_LITERAL_STRING(foo).get()) was an awful antipattern. I'm not sure
> whether Append(NS_LITERAL_STRING(foo)) or AppendLiteral(foo) is better. (Or
> wait until I get review on bug 514173 and use AppendLiteral(MOZ_UTF16(foo)).
> But not Append(MOZ_UTF16(foo)) because that still calls NS_strlen.)

I vote for AppendLiteral(foo), FWIW.
You need to log in before you can comment on or make changes to this bug.