Compare Firefox's file: http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/overrides/appstrings.properties to the "default": http://mxr.mozilla.org/mozilla-central/source/dom/locales/en-US/chrome/appstrings.properties to webapprt's override: http://mxr.mozilla.org/mozilla-central/source/webapprt/locales/en-US/webapprt/overrides/appstrings.properties Why the discrepancy? Are a bunch of webapprt error pages broken? (This fork was initially added in bug 707294.) This came up in bug 624883.

I think it is a mistake.

(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #0) > Why the discrepancy? Are a bunch of webapprt error pages broken? Back when fzzzy added the file in bug 707294, he thought Gecko would fall back to using the default string if it was absent from the webapprt "override", which it doesn't actually do; and I didn't catch that issue in review. Since then he's been focused on moving some strings from dom.properties into appstrings.properties (bug 951441), which has proven to be more complicated than we expected; so we haven't addressed the issue of missing strings in appstrings.properties. We should do so. fzzzy: can you add those strings to webapprt's version of that file?

Myk, since you requested a tracking, I guess you are going to ask for an uplift. could you do it ? thanks :)

Uplift with 17 new strings? I hope not, unless things are utterly broken.

Gavin, what is your opinion on this subject? (uplift or not)

To avoid uplifting the new strings, we could back bug 707294 out from Firefox 29 instead of uplifting this.

(In reply to Marco Castelluccio [:marco] from comment #9) > To avoid uplifting the new strings, we could back bug 707294 out from > Firefox 29 instead of uplifting this. If that's going to happen, please fix the code that require those strings, don't remove them on aurora/beta.

I don't have a strong opinion. I suppose the impact of this is greater than bug 707294 (someone should probably confirm that with testing, though!), so backing that out would probably make sense. (Can't uplift the patch as-is, certainly.)

(In reply to Francesco Lodolo [:flod] from comment #7) > Uplift with 17 new strings? I hope not, unless things are utterly broken. These aren't actually new strings, they're existing strings copied from dom/locales/en-US/chrome/appstrings.properties. So we wouldn't need to translate them, we would just need to copy them from one file to the other. (In reply to Francesco Lodolo [:flod] from comment #10) > If that's going to happen, please fix the code that require those strings, > don't remove them on aurora/beta. No code requires those strings, since they're all overrides of existing strings. Thus we can back out the fix for bug 707294 without making any additional code changes. (In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #11) > I don't have a strong opinion. I suppose the impact of this is greater than > bug 707294 (someone should probably confirm that with testing, though!), so > backing that out would probably make sense. It's a reasonable option, and we can do the necessary testing. flod: from the l10n perspective, which option makes things easier for localizers?

(In reply to Myk Melez [:myk] [@mykmelez] from comment #12) > These aren't actually new strings, they're existing strings copied from > dom/locales/en-US/chrome/appstrings.properties. So we wouldn't need to > translate them, we would just need to copy them from one file to the other. Which sadly is equivalent to "new strings" for localizers. These strings are in a different file, if a localizer doesn't add them they'll fall back to English. > flod: from the l10n perspective, which option makes things easier for > localizers? As I said, the best option is to not touch strings (both deleting or adding). So, if you want to backout bug 707294, that's fine, but please don't backout the strings. Practical example: * Localizer translated the strings in webapprt, you now remove them from Aurora. * Tools remove those strings (tools have usually no memory). * Next cycle they need to localize them again as new. So you'll need a patch that backout code but leave the strings as they are, even if unused.

(In reply to Francesco Lodolo [:flod] from comment #13) > As I said, the best option is to not touch strings (both deleting or adding). > > So, if you want to backout bug 707294, that's fine, but please don't backout > the strings. Ok, here's a patch that stops overriding the strings and packaging the overrides but doesn't back out the strings themselves. I've tested this on the Aurora and Beta branches, and it works as expected: fzzzy's test app shows the original DOM strings instead of the webapprt overrides. If l10n tools only compare source files, then this will back out the changes in bug 707294 that caused this regression without triggering any additional l10n. If the tools also compare packages, however, then presumably the patch will need to continue packaging the overrides, even if it doesn't use them.

Comment on attachment 8379927 [details] [diff] [review] patch to disable webapprt override strings Review of attachment 8379927 [details] [diff] [review]: ----------------------------------------------------------------- Asked Pike to be sure, patch looks good. For comparisongs we rely on /webapprt/locales/l10n.ini, so we compare the content of /webapprt. As long as the files are in there, we're good.

Comment on attachment 8379927 [details] [diff] [review] patch to disable webapprt override strings [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 707294 User impact if declined: Users of webapps in the Web Runtime that trigger certain error pages will see broken error pages. Testing completed (on m-c, etc.): The m-c fix for this bug landed a while ago, but this patch is specific to the Aurora and Beta branches, where we don't want to add any strings. Risk to taking this patch (and alternatives if risky): The patch is low-risk, and the only alternative that fixes the bug is to add strings to the branches, which triggers lots of undesirable late-l10n effort. String or IDL/UUID changes made by this patch: None.

https://hg.mozilla.org/releases/mozilla-aurora/rev/f81828ffe456 https://hg.mozilla.org/releases/mozilla-beta/rev/25f3945e7e7c