We want to remove nsXPIDLString (see bug 1377356). A lot of the uses come from the return values of the methods in nsIStringBundle. We can remove these by changing the return type from |wstring| to |AString|.

dbaron, can you give this a brief look-over to see if it looks like you'd expect? emk, can you please do a normal review. Apologies for the size of the patch.

Comment on attachment 8892858 [details] [diff] [review] Change nsIStringBundle methods to return |AString| instead of |wstring| Yeah, the general approach here looks fine.

Comment on attachment 8892858 [details] [diff] [review] Change nsIStringBundle methods to return |AString| instead of |wstring| Review of attachment 8892858 [details] [diff] [review]: ----------------------------------------------------------------- ::: docshell/base/nsDocShell.cpp @@ +5301,5 @@ > const char16_t* strs[kMaxFormatStrArgs]; > for (uint32_t i = 0; i < formatStrCount; i++) { > strs[i] = formatStrs[i].get(); > } > nsXPIDLString str; Not changed to nsAutoString. ::: layout/base/nsCSSFrameConstructor.cpp @@ +9130,5 @@ > // I can just use that for sized broken images, that can happen, maybe. > void > nsCSSFrameConstructor::GetAlternateTextFor(nsIContent* aContent, > nsIAtom* aTag, > + nsAString& aAltText) Could you remove extra spaces between type names and param names above? ::: layout/base/nsCSSFrameConstructor.h @@ +66,5 @@ > > // get the alternate text for a content node > static void GetAlternateTextFor(nsIContent* aContent, > nsIAtom* aTag, // content object's tag > + nsAString& aAltText); ditto ::: layout/style/ErrorReporter.cpp @@ +330,5 @@ > const char16_t *params[1] = { qparam.get() }; > > nsAutoString str; > sStringBundle->FormatStringFromName(aMessage, > + params, ArrayLength(params), str); params and ArrayLength(params) should move to the previous line. ::: toolkit/mozapps/extensions/AddonContentPolicy.cpp @@ +367,5 @@ > > nsCOMPtr<nsIStringBundle> stringBundle = GetStringBundle(); > > if (stringBundle) { > + rv = stringBundle->FormatStringFromName(aName, aParams, aLength, mError); over 80 columns ::: widget/cocoa/OSXNotificationCenter.mm @@ +329,2 @@ > } > + bundle->GetStringFromName("webActions.settings.label", settingsButtonTitle); over 80 columns

jorgk, I have compiled this but not tested it. The patch is large but mostly formulaic. Most of the changes are one of the following two things. - Removing getter_Copies() calls where necessary from arguments, because we no longer need |char16_t*|-to-|nsAString&| conversions. - Adding ToNewUnicode() calls where necessary for return values, because we now need |nsAString&|-to-|char16_t*| conversions.

Thanks, I've been really busy with fighting bustage all week, so I haven't had a chance to look at this. Apart from the formulaic changes, there seem to be a few other ones, like the removal of NS_Free(kLocalizedInboxName); That's because that's now a nsString nsMsgDBFolder::kLocalizedInboxName; which will take care of its own destruction, right? Please go ahead and land your M-C patch before it rots.

Oh, I meant to say: I'll look at it on Monday the latest.

> Apart from the formulaic changes, there seem to be a few other ones, like > the removal of > NS_Free(kLocalizedInboxName); > That's because that's now a nsString nsMsgDBFolder::kLocalizedInboxName; > which will take care of its own destruction, right? Yes.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e26e9804c545afcb33549da0f7fd693cc3fc671f Bug 1386600 - Change nsIStringBundle methods to return |AString| instead of |wstring|. r=emk,sr=dbaron.

Comment on attachment 8893616 [details] [diff] [review] Update nsIStringBundle method uses to account for them returning |AString| instead of |wstring| Nicholas, thank you so much for helping us in keeping Thunderbird afloat. Preparing the patch would have been a nightmare. I assume you wrote a script for it. I haven't done any testing, but I'll land this now and I'll deal with any fallout later.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/1a7802a4bf9d Update nsIStringBundle method uses to account for them returning |AString| instead of |wstring|. r=jorgk. https://hg.mozilla.org/comm-central/rev/6b0c4e39b0d6 Update nsIStringBundle method uses to account for them returning |AString| instead of |wstring| (Windows-only files). r=me

> I assume you wrote a script for it. Nope! It wasn't mechanical enough for that. But I did use Vim macros to semi-automate it.