After Bug #1018111 landed, we will show the file size too large warning size limit in dialog. Currently the size unit is limited to KB since the limit is usually around few hundreds KB, but it's possible that the limit could over the MB(depends on operator). So we will need to refine this string for different unit support.

Hey Steve Sorry, I couldn't find anyway to handle this as we have to include to include l10n key-value(size) in other l10n key-value . Only way i found is different l10n-key for B, KB,MB (something like this: multimedia-message-exceeded-max-length-B, multimedia-message-exceeded-max-length-KB, multimedia-message-exceeded-max-length-MB). Please help how to fix this. Thanks

There is a similar use case in settings, you can ref this page[1] and the last example about settings: navigator.mozL10n.setAttributes(element, l10nId, { size: fileSize, unit: navigator.mozL10n.get(fileUnit) }); But it should have a problem that the file unit could not be translated after switching the language, and we might need to translate it manually in our localization_helper.js Hi flod, I suspect the localization in js above could not work properly if we don't handle it manaully. Is there any other simplar way to translate file unit before we apply l20n? [1] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/L20n/Localization_Use_Cases

Turning NI to :gandalf, for code related questions they're definitely a better pick.

(In reply to Steve Chung [:steveck] from comment #2) > There is a similar use case in settings, you can ref this page[1] and the > last example about settings: > > navigator.mozL10n.setAttributes(element, l10nId, > { > size: fileSize, > unit: navigator.mozL10n.get(fileUnit) > }); > intially i tried this approach only, but later i came to through julien that apparently l10n people want to get rid of mozl10n.get. Also now there is utils.alert where this code won't work. that's what oleg told (or something like that). Thanks

So, both approaches are acceptable. While it is true that we are trying to deprecate mozL10n.get, we still accept it in several scenarios and referencing one entity as a variable for another is one of them. Saying that, I think that creating three entities here is a preferred way that allows localizers better adjust their string to the context. So I'd recommend: entity-B = {{ value }} B entity-KB = {{ value }} KB entity-MB = {{ value }} MB and then navigator.mozl10n.setAttributes(element, entityId, { value: fileSize}); Also, utils.alert is playing along the API patterns described here: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices#Writing_APIs_that_operate_on_L10nIDs So, you can just: var titleL10n = { id: entityId, args: { value: fileSize} }; utils.alert(titleL10n);

(In reply to kumar rishav (:rishav_) from comment #4) > > intially i tried this approach only, but later i came to through julien that > apparently l10n people want to get rid of mozl10n.get. Also now there is > utils.alert where this code won't work. that's what oleg told (or something > like that). > > Thanks I could image using mozL10n inside utils.alert might have some problem like I said in comment 2. So I'm fine with 3 different entries in comment 1 since gandal also said so.

Hope it's good now.

Comment on attachment 8550325 [details] [diff] [review] Patch So, I see you decided to go for the "add a lot of entities" approach. The code is cleaner, but at the cost of additional work for our localizers. I'd like Flod to approve that. Flod, there's an entity-in-entity dillema here that we cannot resolve properly without l20n format. There are two approaches we can use, one is in the patch, the other is like this: sizeUnitB = {{val}} B sizeUnitKB = {{val}} KB sizeUnitMB = {{val}} MB attached-files-too-large = {[ plural(n) ]} attached-files-too-large[zero] = The file you have selected exceed the {{size}} size limit of a single multimedia message. and then in the code: navigator.mozL10n.ready(function() { var unitL10n = navigator.mozL10n.get('sizeUnit' + unit, {val: val}); navigator.mozL10n.setAttributes(node, 'attached-files-too-large', { 'size': unitL10n, 'n': files }); }); This would mean that we can keep the same number of entities.

Comment on attachment 8550325 [details] [diff] [review] Patch I think this is too expensive for localizers: it definitely has flexibility, but I don't think we need it in case, especially at this cost (21 strings vs 10). Please use the approach suggested by :gandalf in comment 8. It should also be the same approach we're using for downloads (and I wish this common tasks could be centralized like dates) https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/download/download.en-US.properties#L68-L74

Just my 2 cents on this, I don't really like the approach with "navigator.mozL10n.get" in this particular case. Yes, it's probably fine for very simple case like in comment8, pretty ugly, but still reasonable. But here we have something more complex like: "Utils.alert(compositeLabelL10nId).then(actionToPerformOnceAlertIsDismissed)". So alert should not know anything about our way-too-complex-l10nIds and how it's composited. So once language is changed we should close and then reopen alert once again with updated l10nIds - doesn't sound good to me. Modifying simple API like Utils.alert to somehow force update of all its content - isn't right either. IMO it's too expensive in terms of possible errors and regressions. Moreover it's not only about Utils.alert, it can be anything like "blackBox.generateSomeContentWithMyLabel(compositeLabelL10nId)"; where it can be even more complex to update anything on language change. So honestly I think that since l10nId API isn't ideal yet, we as l10nId API consumer should use approach that is as much close to ideal l10nId API (l20n?) as possible. I admit I don't know how much it's expensive for localizers to translate 10 more very similar strings so, please correct me if I'm totally wrong.

(In reply to Oleg Zasypkin [:azasypkin] from comment #10) > I admit I don't know how much it's expensive for localizers to translate 10 > more very similar strings so, please correct me if I'm totally wrong. It is, because translation memory might not catch the string (they're similar, not identical), copy and paste is error prone. In general plural forms are not great for tools, since the same English string is translated in 5 different ways.

So, wrt. Utils.alert, I agree. It should not require refiring. Stas, do you have any other ideas for how to resolve this in 2.2?

Note that it's unlikely a user would change the language when such an alert is displayed. So maybe we should not focus too much on such edge cases.

I agree. Maybe just replace .ready with .once and lets fix it further once we have .l20n format.

Comment on attachment 8550325 [details] [diff] [review] Patch (In reply to Julien Wajsberg [:julienw] from comment #13) > Note that it's unlikely a user would change the language when such an alert > is displayed. So maybe we should not focus too much on such edge cases. I agree that it's edge case but we still need to handle the translation manually. If there is no other flexible solution for this scnerio in 2.2, maybe waiting for l20n is a better option since I don't think having addiontional efforts for localization or code-wise in this patch is necessary.

I agree with steve as we will need this fix when Settings.mmsSizeLimitation will be more (where there is need to display in MB). But in near future, i suppose there is not gonna any change. Till then we will have l20n to handle this.

In that case, this bug is blocked by bug 1027684.

OK, let's revisit once bug 1027684 is resolved.

Comment 8, comment 9 and comment 13 make good points about the allowed use of mozL10n.get here. I wonder why this bug should be blocked by bug 1027684. In comment 16 Kumar says that in the near future there will be no change to mmsSizeLimitation, which I take to mean that we should be good for now and go ahead and fix this bug with the second approach suggested in comment 8. Do we really need to wait for bug 1027684?

Hey Stas, So, here two points that should be considered. 1. As in near future there will be no change to mmsSizeLimitation, (let's say) if even it's there it can be done by 'KB' unit because currently mmsSizeLimitation is 295KB,(let's say) even it will increase, it won't go suddenly beyond 1024KB (or 1000KB). If it go beyond 1000KB, then we will need MB (which rarely possible in coming future). 2.SO i have worked on two patches. This is what you(+L10n team) suggested: https://github.com/mozilla-b2g/gaia/pull/27465/ This is what SMS team suggested: https://github.com/mozilla-b2g/gaia/pull/26966/ Thanks

If you consider point 1, then no need of patch as it's in KB currently.

Note that the constructor can change mmsSizeLimitation himself very easily. I know some change to at least 600KB, I don't know if some change beyond the MB. But TBH I don't really care, as the dialog is readable even with KB. That's why I don't mind waiting for a clean way to do this. But I also don't mind using "get" here. So we might as well move forward. What do you think, Steve ?

(In reply to Julien Wajsberg [:julienw] from comment #22) > Note that the constructor can change mmsSizeLimitation himself very easily. > I know some change to at least 600KB, I don't know if some change beyond the > MB. > > But TBH I don't really care, as the dialog is readable even with KB. That's > why I don't mind waiting for a clean way to do this. > > But I also don't mind using "get" here. So we might as well move forward. > What do you think, Steve ? As far as I know some operator don't have the mms size limitation, but just like you said it's still readable with KB. That's why we want to wait for proper architecture landed. I'm fine if we apply "get" here, but we might need to add lisener for sizeUnit in LocalizationHelper for this solution to prevent the unit not translated. It will nedd a liitle more effort while localization changed but I don't want leave any regression in master. wdyt?

Clearing the needinfo since this isn't really my call to make. I'll leave the decision to Julien and Steve.

Technical decisions aside, we still have poor English as reported in bug 1018111 comment 28, and we're approaching string freeze in a in 4 days.

Right flod, we should have filed a quick follow-up bug for this issue alone. I can do that.

(The issue reported by flod has been fixed in bug 1134178)

(In reply to Steve Chung [:steveck](OOO 2/18 - 2/23) from comment #23) > (In reply to Julien Wajsberg [:julienw] from comment #22) > > Note that the constructor can change mmsSizeLimitation himself very easily. > > I know some change to at least 600KB, I don't know if some change beyond the > > MB. > > > > But TBH I don't really care, as the dialog is readable even with KB. That's > > why I don't mind waiting for a clean way to do this. > > > > But I also don't mind using "get" here. So we might as well move forward. > > What do you think, Steve ? > > As far as I know some operator don't have the mms size limitation, but just > like you said it's still readable with KB. That's why we want to wait for > proper architecture landed. > > I'm fine if we apply "get" here, but we might need to add lisener for > sizeUnit in LocalizationHelper for this solution to prevent the unit not > translated. It will nedd a liitle more effort while localization changed but > I don't want leave any regression in master. wdyt? I don't think the work is worth it. This is a short-lived panel, I don't mind if that it's not translated if the user happens to change the language while the panel is displayed. I don't think this will happen anywhere.

Agree. While I believe there's a benefit of keeping our UI async and retranslatable that goes beyond language switching, this bug is specific. Our L10n API does not have a good solution yet, so we'd need a dirty hack and would not get the best value out of it. We will land updated l10n API this quarter and we can then solve it properly.

Hi Zibi, Do we have solution for this now? Thanks

Hi Steve, Any update for this bug? Thanks

Sorry for the delay in responses. We are actually moving forward with two technologies that will be able to help: - l20n wise, we are moving apps currently to l20n.js which supports L20n format. We still need to enable a few more features within l20n file format to support this, but we're close. - Intl API - we're extending Intl API with UnitFormat - https://github.com/tc39/ecma402/issues/32

Zibi answered. So clearing the ni?

Unassigning, as it's under development.

Mass closing of Gaia::SMS bugs. End of an era :(

Mass closing of Gaia::SMS bugs. End of an era :(