GNOME Bugzilla – Bug 760391
Using gettext and ngettext on the same string can cause %d to be shown in the user interface
Last modified: 2016-09-20 08:15:55 UTC
In src/selectionbar.vala, function update_open_btn: // Translators: This is a button to open box(es) in new window(s) if (items == 0) open_btn.label = _("_Open in new window"); else open_btn.label = ngettext ("_Open in new window", "_Open in %d new windows", items).printf (items); When items is zero, it uses gettext instead of ngettext. This causes problems for languages that only has one form (Plural-Forms: nplurals=1; plural=0;) because the only translated form must contain %d, so open_btn.label become a user-visible string that contains %d.
Created attachment 318644 [details] gnome-boxes screenshot in zh_TW
Created attachment 321889 [details] [review] selectionbar: Don't use the same message in both gettext and ngettext In languages that only have one plural form, we cannot use the translated message in both string literal and printf format string because it can cause '%d' to be shown in the user interface. This patch also remove the mnemonic because it is not useful for an insensitive button.
I hope this bug can be fixed before GNOME 3.20 string freeze, so translators can translate this new string.
Review of attachment 321889 [details] [review]: Thanks for the patch. Also thanks for providing a lot of details in the log. \o/ Although please try to keep the shortlog/summary line short (50 chars ideally): https://wiki.gnome.org/Git/CommitMessages . But don't worry too much about this, I can help you out on this but do try. :) ::: src/selectionbar.vala @@ +152,2 @@ if (items == 0) + open_btn.label = _("Select boxes to open them"); No, we don't want to show this string on a button. Can't we just use not check for items == 0 and just use ngettext to always translate (like in the else block)? @@ +153,3 @@ + open_btn.label = _("Select boxes to open them"); + // Translators: This is a button to open box(es) in new window(s) when + // at least one box is selected. You need to move this 1 line below (just under else and indented).
Review of attachment 321889 [details] [review]: ::: src/selectionbar.vala @@ +152,2 @@ if (items == 0) + open_btn.label = _("Select boxes to open them"); I was informed by afranke and amigadave that we don't need to special case 'items == 0' here and just use ngettext.
(In reply to Zeeshan Ali (Khattak) from comment #5) > Review of attachment 321889 [details] [review] [review]: > > ::: src/selectionbar.vala > @@ +152,2 @@ > if (items == 0) > + open_btn.label = _("Select boxes to open them"); > > I was informed by afranke and amigadave that we don't need to special case > 'items == 0' here and just use ngettext. Doesn't this cause 'Open in 0 new windows' to be shown in languages having only one form?
(In reply to Ting-Wei Lan from comment #6) > Doesn't this cause 'Open in 0 new windows' to be shown in languages having > only one form? Using ngettext and ngettext only ensures every language gets exactly what they need. If you try to be smarter than that, you're actually doing more harm than good.
Created attachment 322568 [details] [review] selectionbar: msgid cannot be shared in literal and printf In languages that only have one plural form, we cannot use the translated message in both string literal and printf format string because it can cause '%d' to be shown in the user interface. This patch removes the zero check, so ngettext and printf are always used and '%d' is always replaced by the correct number.
(In reply to Ting-Wei Lan from comment #8) > Created attachment 322568 [details] [review] [review] > selectionbar: msgid cannot be shared in literal and printf > > In languages that only have one plural form, we cannot use the > translated message in both string literal and printf format string > because it can cause '%d' to be shown in the user interface. > > This patch removes the zero check, so ngettext and printf are always > used and '%d' is always replaced by the correct number. Actually ngettext doesn't handle zero items case like 1 items case. While I very much appreciate the fact that you not only filed the bug but also provided patches, please do test your patch before submitting. If you don't have means to do that easily, please do tell others that patch is untested.
Created attachment 322655 [details] Screenshot of ngettext showing "Open in 0 windows"
Yes, I did test it before submitting and I was not sure whether 'Open in 0 windows' is acceptable. If we don't make zero special, is it possible to do better than this?
Created attachment 322656 [details] [review] selectionbar: Add a translation context Add context to the button label for the case of "0 boxes selected". -- How about this approach instead?
(In reply to Ting-Wei Lan from comment #11) > Yes, I did test it before submitting and I was not sure whether 'Open in 0 > windows' is acceptable. Ah good! No, to me at least it sounds just wrong. >If we don't make zero special, is it possible to do > better than this? Yeah, I think so. See the attached patch.
(In reply to Zeeshan Ali (Khattak) from comment #13) > (In reply to Ting-Wei Lan from comment #11) > >If we don't make zero special, is it possible to do > > better than this? > > Yeah, I think so. See the attached patch. Attachment 322656 [details] keeps zero special and I think it is a conflict with comment #5, but I don't care whether the if check is kept as long as the solution is accpetable to you and the patch works. I ran 'make -C po/ update-po' and edited this newly added item in po/zh_TW.po: #: ../src/selectionbar.vala:152 #, fuzzy msgctxt "0 items selected" msgid "_Open in new window" msgstr "在新視窗中開啟(_O)" However, this change wasn't included in the generated po/zh_TW.gmo file, and running gnome-boxes with zh_TW locale showed English text instead of the msgstr I added.
(In reply to Ting-Wei Lan from comment #14) > (In reply to Zeeshan Ali (Khattak) from comment #13) > > (In reply to Ting-Wei Lan from comment #11) > > >If we don't make zero special, is it possible to do > > > better than this? > > > > Yeah, I think so. See the attached patch. > > Attachment 322656 [details] keeps zero special and I think it is a conflict > with comment #5, Yeah, I'd just prefer the simplest solution. > but I don't care whether the if check is kept as long as > the solution is accpetable to you and the patch works. Good. :) > I ran 'make -C po/ update-po' and edited this newly added item in > po/zh_TW.po: > > #: ../src/selectionbar.vala:152 > #, fuzzy > msgctxt "0 items selected" > msgid "_Open in new window" > msgstr "在新視窗中開啟(_O)" > > However, this change wasn't included in the generated po/zh_TW.gmo file, and > running gnome-boxes with zh_TW locale showed English text instead of the > msgstr I added. Yeah I now tested with zh_TW lang and I'm getting the same. :(
Created attachment 322689 [details] [review] selectionbar: Don't special case '0 boxes' case In languages that only have one plural form, we cannot use the translated message in both string literal and printf format string because it can cause '%d' to be shown in the user interface. This patch changes number of items from 0 to 1 so that ngettext shows the same text for both cases, which was the intention of this code.
Review of attachment 322689 [details] [review]: ::: src/selectionbar.vala @@ -149,1 +149,3 @@ open_btn.sensitive = items > 0; + // We want to show an unsensitive button, but not with the "Open in 0 new windows" string + items = 1; Isn't it wrong to always set items to 1? In zh_TW locale it causes 1 to be shown regardless the number of selected items. Even if I change it to 'items = items > 0 ? items : 1;', it is still wrong because 1 is shown when no items is selected.
Review of attachment 322689 [details] [review]: ::: src/selectionbar.vala @@ -149,1 +149,3 @@ open_btn.sensitive = items > 0; + // We want to show an unsensitive button, but not with the "Open in 0 new windows" string + items = 1; ngettext is supposed to show the first string for singular cases. Is it very wrong to show that if your locale's translation to "Open in new window" is more like "Open in 1 new windows"?
In zh_TW there is no difference between singular and plural cases. There is only one form and it is always like "Open in %d new windows".
(In reply to Ting-Wei Lan from comment #19) > In zh_TW there is no difference between singular and plural cases. There is > only one form and it is always like "Open in %d new windows". That's what I thought, hence my question that if it's really that wrong to show "Open in 1 new window" if no items are selected? The button is disabled anyway so user won't be confused?
The number is started at 1, and users may assume it means only 1 item should be selected although they know they are wrong when more items are selected. It is also odd to see the number not going down when the last selected item is unselected.
Had a long chat about this on IRC with other GNOME devs: ---------- <zeenix> mclasen: i'm stumped by bug#760391 <zeenix> mclasen: you sure showing "Launch in 0 new windows" is fine? <mclasen> if translators don't think that is fine, it is in their power to change it <mcatanzaro> zeenix: "Open in 0 new windows" is not fine :) <mclasen> I just don't get how this case even arises <mclasen> why offer opening in the first place if 0 items are selected ? <zeenix> mclasen: it's a button in selection bar <mclasen> why is that button sensitive when there is no selection ? <teuf> mclasen: it's a grayed out button <zeenix> it's shown when you hit the 'selection mode' button <zeenix> it is *not* sensitive <mclasen> and it has a label that says "Open in 0 new windows" ? <teuf> mclasen: the goal is to show "Open in new window" grayed out (or some sensical string) <teuf> then you select one VM, it becomes sensitive and still says teh same <teuf> you select 2, it switches to plural <zeenix> ngettext ("Open in new windows", "Open in %d new windows", n_selected); <zeenix> that shows the latter for 0 items for some reason * mclasen would just make the button say "Open in new windows" <mclasen> no need for counting the windows, is there ? <teuf> zeenix: yup, 0 items is valid in english, this is explained in gettext documentation somewhere <teuf> (about plurals) <teuf> zeenix: French gets it right! <zeenix> teuf: valid yes but sounds stupid :) <zeenix> jimmac: aday: not sure on whose design i based the title for this button but what do you think is better? <teuf> zeenix: the details are https://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html#Plural-forms <zeenix> jimmac: aday: shall I just drop showing the number (like mclasen suggested)? <mcatanzaro> zeenix: 0 is plural in English and many other languages <mclasen> you still need to differentiate between selection size 1 and != 1, I'm afraid <zeenix> mcatanzaro: hmm.. true <mcatanzaro> zeenix: You could just have two strings? /* TRANSLATORS: label on insensitive button where 0 items are selected*/ _("Open in new window") * Kekun (Kekun@fac34-h03-176-151-16-46.dsl.sta.abo.bbox.fr) has joined #gnome-hackers <zeenix> mcatanzaro: that's what the code currently tries to do * jrb has quit (Ping timeout: 184 seconds) <mcatanzaro> And: /* TRANSLATORS: label on button when %d windows are selected */ ngettext ("Open in new window", "Open in %d new windows", n_selected); <-- note "open in new window" singular <zeenix> mcatanzaro: and hence the bug <zeenix> mcatanzaro: for some reason C_ doesn't work :( <zeenix> mcatanzaro: and we want the same label for 0 and 1 items <mcatanzaro> Ah, I understand now... crap! <mclasen> still, take out the %d - no need to count hte windows <mcatanzaro> zeenix: I think, just add third string for 1 <mcatanzaro> Or do what mclasen says <mclasen> zeenix: what do you mean C_ doesn't work ? <teuf> zeenix: Ting-Wei seems to disagree with having the same string for 0 and 1 (that zh_TW it would be odd) <mcatanzaro> zeenix: But it'd be OK to have hardcoded translations for 0, 1, and then ngettext, IMO, then the first string passed to ngettext would be used in Chinese but not English. <mclasen> you should look at what ends up in the .pot file, that will tell you what is going wrong (if anything) <mcatanzaro> lantw44: ^ <mcatanzaro> zeenix: That would be: /* TRANSLATORS: label on button when %d windows are selected */ ngettext ("Open in %d new window", "Open in %d new windows", n_selected); <mcatanzaro> Which is kind of confusing, maybe you should listen to mclasen :) * hughsie is now known as hughsie-afk <zeenix> mclasen: https://bugzilla.gnome.org/show_bug.cgi?id=760391#c14 <bugbot> Bug 760391: gnome-boxes, normal, Normal, 3.18, gnome-boxes-maint, NEW , Using gettext and ngettext on the same string can cause %d to be shown in the user interface <zeenix> and c#15 toot <zeenix> too <zeenix> mclasen: pot file is generated correctly with the context but someohow the special case string with context isn't used * mclasen suggests operator error <zeenix> oh actually, the original English string is shown <radusqrt> hello! i've encountered a problem trying to build gnome-calendar at step 46/47. can somebody help me? <zeenix> mclasen: don't see how. Both Ting-Wei and I were able to reproduce the issue easily <mcatanzaro> mcrha: ^ Looks like e-d-s is broken in jhbuild :( <mcatanzaro> mcrha: Do you want to just build it with --disable-google-auth or...? <mclasen> a reproducer thats plain c and ideally part of the glib tests is required if you want me to look further <mcatanzaro> Ah it depends on webkitgtk-3.0 <zeenix> mclasen: ok, I'll go with the context patch for now and create a new bug for C_ not working properly * mclasen knows for a fact that C_ is working properly in many places ---------- Based on this chat, I'll go with attachment#322656 [details] and look into the issue we were both seeing as a separate bug.
Attachment 322656 [details] pushed as d12a85e - selectionbar: Add a translation context
(In reply to Ting-Wei Lan from comment #14) > #: ../src/selectionbar.vala:152 > #, fuzzy > msgctxt "0 items selected" > msgid "_Open in new window" > msgstr "在新視窗中開啟(_O)" > > However, this change wasn't included in the generated po/zh_TW.gmo file, and > running gnome-boxes with zh_TW locale showed English text instead of the > msgstr I added. I found the problem is caused by the fuzzy attribute added when running update-po. After removing it and rebuilding the gmo file, the Chinese message for 0 items successfully shows when no item is selected. I will update zh_TW translation if I have time.