775142 - java.util.UnknownFormatConversionException: Conversion: at java.util.Formatter$FormatSpecifierParser.unknownFormatConversionException(Formatter.java) mainly with Japanese and Chinese builds

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Scoobidiver (away)]

There's one crash in 16.0a2/20120717: bp-aab81d4b-54eb-4267-85f0-43bb72120718.

java.util.UnknownFormatConversionException: Conversion: 
	at java.util.Formatter$FormatSpecifierParser.unknownFormatConversionException(Formatter.java:2304)
	at java.util.Formatter$FormatSpecifierParser.advance(Formatter.java:2298)
	at java.util.Formatter$FormatSpecifierParser.parseConversionType(Formatter.java:2377)
	at java.util.Formatter$FormatSpecifierParser.parseArgumentIndexAndFlags(Formatter.java:2348)
	at java.util.Formatter$FormatSpecifierParser.parseFormatToken(Formatter.java:2281)
	at java.util.Formatter.doFormat(Formatter.java:1069)
	at java.util.Formatter.format(Formatter.java:1040)
	at java.util.Formatter.format(Formatter.java:1009)
	at java.lang.String.format(String.java:1998)
	at android.content.res.Resources.getString(Resources.java:373)
	at android.content.Context.getString(Context.java:295)
	at org.mozilla.gecko.BrowserToolbar.updateTabCount(BrowserToolbar.java:372)
	at org.mozilla.gecko.BrowserToolbar.refresh(BrowserToolbar.java:559)
	at org.mozilla.gecko.BrowserToolbar.onTabChanged(BrowserToolbar.java:302)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:366)
	at org.mozilla.gecko.Tabs.notifyListeners(Tabs.java:357)
	at org.mozilla.gecko.GeckoApp$3.run(GeckoApp.java:844)
	at android.os.Handler.handleCallback(Handler.java:605)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:137)
	at android.app.ActivityThread.main(ActivityThread.java:4517)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:993)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:760)
	at dalvik.system.NativeStart.main(Native Method)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.util.UnknownFormatConversionException%3A+Conversion%3A+at+java.util.Formatter%24FormatSpecifierParser.unknownFormatConversionException%28Formatter.java%29

Comment 1

[Scoobidiver (away)]

It's #3 top crasher in 18.0a2 and #14 in 19.0a1.

There's no stack trace because of bug 800552.

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.util.UnknownFormatConversionException%3A%20Conversion%3A%20%E3%81%A7%20at%20java.util.Formatter%24FormatToken.unknownFormatConversionException%28Formatter.java%29

Comment 2

[Scoobidiver (away)]

It's currently #1 top crasher in 18.0a2 and #3 in 19.0a1.

Comment 3

[Scoobidiver (away)]

It accounts for 23% of crashes in 18.0a2 and 5% in 19.0a1.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

This is a bug in l10n. Some of the format specifiers are not preserved in the translations. For example, running a grep search in the Aurora l10n,

> eu: <!ENTITY suggestions_prompt "&#037;(r)en bilaketa-gomendioak gaitu nahi dituzu?">

> ff: <!ENTITY num_tabs "tabbe &#037;">

> gd: <!ENTITY suggestions_prompt "A bheil thu ag iarraidh molaidhean luirg o &#037;?">

> ja-jp-mac: <!ENTITY suggestions_prompt "&#037; で検索語句の候補を表示しますか？"> (AFAICT, this is only used on Macs, so we probably don't need this for Fennec)

> ja: <!ENTITY suggestions_prompt "&#037; で検索語句の候補を表示しますか？">

> zh-TW: <!ENTITY suggestions_prompt "您想要開啟 &#037; 的搜尋建議嗎？"> (there also seems to be a lot of empty spaces at the end of this string)

You can see that the trailing letters in the specifiers "&#037;d" and "&#037;s" are left out. This affects m-c, aurora, and beta. (And maybe release)

Comment 5

[Robert Kaiser]

(In reply to Jim Chen [:jchen :nchen] from comment #4)
> You can see that the trailing letters in the specifiers "%d" and
> "%s" are left out. This affects m-c, aurora, and beta. (And maybe
> release)

Well, in HTML or XML, which DTDs are formed after, an entity starts with & and ends with ; so I guess that's what people are following here as well. If stuff here is acting differently, maybe it's wrong to use a DTD in the first place?

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/locales/en-US/android_strings.dtd#243

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

based on comment 4, change the language and turning on a search site's suggestion may reproduce the engine if I understand the situation correctly?

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Robert Kaiser (:[e-mail]) from comment #5)
> (In reply to Jim Chen [:jchen :nchen] from comment #4)
> > You can see that the trailing letters in the specifiers "%d" and
> > "%s" are left out. This affects m-c, aurora, and beta. (And maybe
> > release)
> 
> Well, in HTML or XML, which DTDs are formed after, an entity starts with &
> and ends with ; so I guess that's what people are following here as well. If
> stuff here is acting differently, maybe it's wrong to use a DTD in the first
> place?

We are using C-style format specifiers (e.g. %d which becomes %d) in these strings, so any translation must remember to keep the letter after the percent sign. Otherwise we get this exception when we feed the string to the format function.

(In reply to Naoki Hirata :nhirata from comment #7)
> based on comment 4, change the language and turning on a search site's
> suggestion may reproduce the engine if I understand the situation correctly?

Yes, I could reproduce with the language set to Traditional Chinese (zh-TW) using a multi-locale Nightly. Fennec crashes as soon as you open the awesome screen, because the suggestions prompt is shown by default when you first run Fennec.

Comment 9

[Robert Kaiser]

(In reply to Jim Chen [:jchen :nchen] from comment #8)
> We are using C-style format specifiers (e.g. %d which becomes %d) in
> these strings, so any translation must remember to keep the letter after the
> percent sign. Otherwise we get this exception when we feed the string to the
> format function.

Ugh, this is completely surprising in a DTD file, no wonder localizers get confused. Is there at least an L10n notice in this file for that? I'd feel better we had stuff like this in a .properties file, where things like that are somewhat more expected (but still need a L10n note).

This is kind of a trap we set for localizers, we need to make sure we can lead them out of it or avoid it completely.

Comment 10

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

(In reply to Robert Kaiser (:[e-mail]) from comment #9)
> (In reply to Jim Chen [:jchen :nchen] from comment #8)
> > We are using C-style format specifiers (e.g. %d which becomes %d) in
> > these strings, so any translation must remember to keep the letter after the
> > percent sign. Otherwise we get this exception when we feed the string to the
> > format function.
> 
> Ugh, this is completely surprising in a DTD file, no wonder localizers get
> confused. Is there at least an L10n notice in this file for that? I'd feel
> better we had stuff like this in a .properties file, where things like that
> are somewhat more expected (but still need a L10n note).

This is an Android Java string file, not a Gecko file. We can't move it to a .properties file, but we could add L10N notes.

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

STR:
1. remove previous version of nightly
2. set device to traditional chinese
3. install multilocal nightly (http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-central-android/)
4. launch nightly
5. tap url bar

Expected: no crash; awesome bar
Actual: crash

(Japanese is not translated in the nightly so it doesn't crash on nightly, having said that I think this crashes on aurora 18a2 w/ Japanese?)

Comment 12

[Robert Kaiser]

(In reply to Mark Finkle (:mfinkle) from comment #10)
> This is an Android Java string file, not a Gecko file. We can't move it to a
> .properties file, but we could add L10N notes.

Wow, a Java string file mimicing DTD format and expecting a construct like %d would have surely surprised me as a localizer (and that's where I started as a contributor and am still found at times).

I wonder if the error checker that Axel runs on the L10n dashboard can be made aware of things like that so it makes this visible to localizers.

Comment 13

[Alex Keybl [:akeybl]]

mfinkle - can you reassign now that we have reproducible steps?

Comment 14

[Alex Keybl [:akeybl]]

(an engineering fix would be preferable, if at all possible, to prevent future l10n issues like this)

Comment 15

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

(In reply to Alex Keybl [:akeybl] from comment #14)
> (an engineering fix would be preferable, if at all possible, to prevent
> future l10n issues like this)

We are discussing an engineering solution, but the downside is a bad string will be displayed. We do need to fix these bad strings too.

Comment 16

[Robert Kaiser]

I have sent out a message to the dev-l10n list/group about this, citing comment #4 in full. I hope this will take care of Beta 18 and Aurora 19, but an engineering solution in Nightly 20 would be nicer long-term.

Comment 17

[Brian Nicholson (:bnicholson)]

Attachment: Use placeholder format strings for localization

This patch extracts the C-style format strings into their own entities. We can then use standard entity references in the strings themselves, which will hopefully prevent future errors like this.

Comment 18

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

Comment on attachment 683782 [details] [diff] [review]
Use placeholder format strings for localization

Aren't you going to need to update strings.xml.in ?

Comment 19

[Axel Hecht [:Pike]]

Comment on attachment 683782 [details] [diff] [review]
Use placeholder format strings for localization

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

I like the idea, but you can move the entity definitions into strings.xml.in, so there's not going to be any chance of confusion or bustage in that one string.

Also, yes, the two strings affected will need to get the ID bumped in android_strings.dtd and strings.xml.in.

Comment 20

[Brian Nicholson (:bnicholson)]

Attachment: Use placeholder format strings for localization

Bumped IDs in strings.xml.in and moved format strings.

Comment 21

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

Comment on attachment 684118 [details] [diff] [review]
Use placeholder format strings for localization

Don't you need to change some Java code?

Comment 22

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

Comment on attachment 684118 [details] [diff] [review]
Use placeholder format strings for localization

Oh. No you don't \o/

Comment 23

[Scoobidiver (away)]

It seems urgent to land the patch if Chinese and Japanese localizers haven't fixed the bad translation unless Axel does it across all channels (you don't need to know the language).

Comment 24

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/cba096382a39

This patch doesn't actually fix any crashes; it simply reduces the chance that translators will make this same mistake in the future. These string files still need to be updated to fix the crashes.

Comment 25

[Axel Hecht [:Pike]]

Actually, this path does fix the crashes, as the crashing strings aren't picked up by the build no more. They'll be in English until the localizers catch up with the change, of course. Better than crashing, too.

Comment 26

[Scoobidiver (away)]

I doubt this patch will be uplifted to Beta because it impacts l10n strings for every locales while fixing the Japanese and Chinese suggestions_prompt string is easy to do by someone who have commit rights.

Comment 27

[Robert Kaiser]

(In reply to Axel Hecht [:Pike] from comment #25)
> Actually, this path does fix the crashes, as the crashing strings aren't
> picked up by the build no more. They'll be in English until the localizers
> catch up with the change, of course. Better than crashing, too.

I fully agree in terms of trunk. As Scoobidiver says, this probably won't make Beta or Aurora, though, and the problem exists in both as well. What can we do there to fix this?

Comment 28

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cba096382a39

Comment 30

[Cristian Nicolae (:xti)]

I cannot reproduce this crash performing the STR from comment #11. Closing bug as verified fixed on:

Firefox 20.0a1 (2012-11-28)
Device: Galaxy Nexus
OS: Android 4.1.1

Comment 31

[Scoobidiver (away)]

Japanese builds seem unaffected in 18.0b1 according to crash stats but are in 19.0a2 despite the suggestions_prompt string is the same.

Comment 32

[Alex Keybl [:akeybl]]

(In reply to Scoobidiver from comment #31)
> Japanese builds seem unaffected in 18.0b1 according to crash stats but are
> in 19.0a2 despite the suggestions_prompt string is the same.

Given that, untracking for FF18.