622429 - Italian strings need apostrophes to be escaped

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Android multi-locale builds are failing:

04:24:40     INFO -  /tools/android-sdk/platforms/android-8/tools/aapt package -f -M AndroidManifest.xml -I /tools/android-sdk/platforms/android-8/android.jar  -S res -F gecko.ap_
04:24:40     INFO -  res/values-it/strings.xml:24: error: Apostrophe not preceded by \ (in Si è verificato un errore durante il caricamento di un file necessario per l'esecuzione di Fennec)
04:24:40     INFO -  res/values-it/strings.xml:29: error: Apostrophe not preceded by \ (in Invia una segnalazione dell'arresto anomalo a Mozilla)
04:24:40     INFO -  res/values-it/strings.xml:30: error: Apostrophe not preceded by \ (in Includi l'indirizzo della pagina)
04:24:40     INFO -  make[3]: *** [gecko.ap_] Error 1


Looks like the "'" needs to be escaped with a "\"

Comment 1

[Axel Hecht [:Pike]]

Where is that coming from?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1293966245.1293971479.14441.gz&fulltext=1

Comment 3

[Axel Hecht [:Pike]]

Let me rephrase that:

As per XML standards, the ampersand doesn't need to be escaped.

So, what's complaining, and suggesting to do things that make no sense in XML files?

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #3)
> Let me rephrase that:
> 
> As per XML standards, the ampersand doesn't need to be escaped.
> 
> So, what's complaining, and suggesting to do things that make no sense in XML
> files?

http://developer.android.com/guide/topics/resources/string-resource.html#FormattingAndStyling

Must be something about how the string is post processed from the XML file.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 500738 [details] [diff] [review]
patch

We are only adding escape chars, not changing the text in any way. This will get a nightly build too (or we hope it will).

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch to drop IT

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 500742 [details] [diff] [review]
patch to drop IT

this seems like the right way to drop a locale, based on:

http://hg.mozilla.org/build/mozharness/file/bfd8318f90d6/scripts/multil10n.py#l343
and
http://hg.mozilla.org/build/mozharness/file/bfd8318f90d6/configs/multi_locale/trunk_android.json#l6

And we can't land the real fix in the "it" repo cause of permissions. We can turn "it" back on when it is fixed.

Comment 9

[Francesco Lodolo [:flod]]

This is so wrong that I don't even know where to start... For example, this breaks all localization tools and I have to manage this file by hand.

BTW there should be a bug about fixing the real problem, not the consequence: if you're using entities in dtd files you must expect to find non escaped apostrophes. 

At least, is it ok to use typographic apostrophes?
http://hg.mozilla.org/l10n-central/it/rev/f4c8aa839cac

Comment 10

[Axel Hecht [:Pike]]

Typographic apostrophes probably work, yes.

That said, we can't wallpaper over that platform deficiency by removing locales for the rest of our lives.

I've been pondering a few ideas:

- drop using a dtd (as the result isn't really XML, but just weird strings in an XML container)
-- downside is, what to replace it with? Also, the tool to create the xml container just gets more messy
- drop shipping on android
-- not gonna fly with the users
- enhancing compare-locales to know about crufty extra rules for android
-- we'd have to do this anyway, I guess
-- it's non-trivial, and is adding some weird assumptions on what android files would be.

The sad story is, we can't quickly fix compare-locales. It's a non-trivial patch, and turning on more errors has severe implications on the rest of our releases (aka, anything needs to ship with l10n-merge on, down to stable releases of thunderbird and seamonkey). I hope that we'd be able to actually deploy stuff on the l10n-dashboard soon again (we can't right now, no access to the machine anymore).

Long term, there's good and bad news: Good news, we can probably just implement l20n for java and drop android's confused in-house crap. Bad news, that'd still be java.

Side remark, mfinkle, I hope you enjoy as much as I do to run across a more hacky l10n-infra in android than in gecko ;-)

Comment 11

[Francesco Lodolo [:flod]]

If typographic apostrophes are ok, can someone re-enable Italian builds?

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch to add IT

Adds IT back since the strings were fixed in the IT repo

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed: http://hg.mozilla.org/mobile-browser/rev/6428efdf5077

closing bug

Comment 14

[Aakash Desai [:aakashd]]

verified FIXED on build:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b9pre) Gecko/20100109 Namoroka/4.0b9pre Fennec/4.0b4pre (multi-locale)