1155901 - [l10n] The "Call Ended" string is truncated in some languages

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Yeojin Chung [:YeojinC]]

Attachment: CallEnded.png

Description:
In some languages, the string "Call Ended" on the callscreen is truncated. The string used to run off the screen, which was filed on bug 1142485. Now the font size is reduced, so majority of the languages can fit within the screen properly. However, languages such as Malay, Tamil, and Bosnian displays the truncation issue.

Repro Steps:
1) Update a Flame to 20150417010203.
2) Set the device language in Malay, Tamil, or Bosnian from Settings > Language.
3) Receive an incoming call, and answer the call.
4) End the phone call.
5) Observe the call screen.

Actual:
The "Call Ended" string is truncated.

Expected:
The "Call Ended" string is properly translated without truncation.

Environmental Variables:
Device: Flame 3.0  (KK, 319mb, full flash)
Build ID: 20150417010203
Gaia: 3cd0a9facce26c2acc7be3755a17131a6358e33f
Gecko: 51e3cb11a258
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Repro frequency: 5/5
See attached: screenshot

Comment 1

[Yeojin Chung [:YeojinC]]

This issue also reproduces on Flame 2.2.

Result: The "Call Ended" string is truncated.

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150417002504
Gaia: d50b8a3919a7b4d8d289f150d3b9bed704ebafa9
Gecko: 5ebf32030512
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 2

[Delphine Lebédel [:delphine - Use needinfo]]

Adding the regression keyword, since this was supposed to be fix in Bug 1142485

Comment 3

[KTucker [:KTucker][Inactive 3/4/2016]]

Normally, we don't add the keywords regression and regression-window wanted when each branch is affected. However, I will have the tester test the builds after Bug 1142485 was resolved as fixed to see if this was indeed fixed and broke again.

Comment 4

[Delphine Lebédel [:delphine - Use needinfo]]

Woops my bad :) TGIF. Thanks Kevin

Comment 5

[Brogan Zumwalt [Inactive]]

B2G-Inbound Regression Window:

Tamil and Melay language truncation seen

Last working B2G-Inbound:
Device:  Flame 3.0
BuildID: 20150326041844
Gaia: 1aec7e584f1b6d6bfe5b1c2afe1985b3e64c84ef
Gecko: d51f34c951cd
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

First broken B2G-Inbound:
Device: Flame 3.0
Build ID: 20150326044444
Gaia: 9db90f94b29779be08bedcf178fdaa32c0e6a118
Gecko: 30236ca212cb
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0


Working Gaia with Broken Gecko issue does NOT reproduce:
Gaia: 1aec7e584f1b6d6bfe5b1c2afe1985b3e64c84ef
Gecko: 30236ca212cb

Working Gecko with Broken Gaia issue DOES reproduce:
Gaia: 9db90f94b29779be08bedcf178fdaa32c0e6a118
Gecko: d51f34c951cd


B2G-Inbound Pushlog:
https://github.com/mozilla-b2g/gaia/compare/1aec7e584f1b6d6bfe5b1c2afe1985b3e64c84ef...9db90f94b29779be08bedcf178fdaa32c0e6a118


Issue appears to have been caused by changes made in bug 1142485

Note: Oddly we start seeing Bosnian cyrilic language affected in this later range instead of the above range: https://github.com/mozilla-b2g/gaia/compare/9c97666d8b835d451ec04f05aa0efa9ac5c3f0c7...16383ec2bf3ed46f893b15b3fab2892e9fadc4e7

Comment 6

[KTucker [:KTucker][Inactive 3/4/2016]]

Gabriele, can you take a look at this please? This might have been caused by the work done for bug 1142485.

Comment 7

[Gabriele Svelto [:gsvelto]]

(In reply to KTucker [:KTucker] from comment #6)
> Gabriele, can you take a look at this please? This might have been caused by
> the work done for bug 1142485.

Yes, taking this. In bug 1142485 we decided not to go for a resizable field in the hope that the new size would be enough to fit all the languages but apparently this is not the case. I'll replace the call ended field using a variable-font-size one.

The fix for bug 1154634 is essentially the same.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] gabrielesvelto:bug-1155901-resize-call-ended-string > mozilla-b2g:master

Comment 9

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8595372 [details] [review]
[gaia] gabrielesvelto:bug-1155901-resize-call-ended-string > mozilla-b2g:master

This patch resizes the call ended string automatically to fit within the maximum size we've assigned to its element. The font size of the duration is also adjusted accordingly so that it doesn't look out of place; this cannot cause both elements to be too large as we'll be shrinking the duration but never growing it past the established maximum font size (2.7rem).

I'm using the new style L10n methods to handle this and I'll file a follow up if we don't have one yet to get rid of LazyL10n and use proper, non-deprecated l10n functionality.

Comment 10

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8595372 [details] [review]
[gaia] gabrielesvelto:bug-1155901-resize-call-ended-string > mozilla-b2g:master

Sorry, this isn't ready for review yet. A unit-test is broken and it seems like it isn't working properly for Tamil (though Bosnian does work fine now).

Comment 11

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8595372 [details] [review]
[gaia] gabrielesvelto:bug-1155901-resize-call-ended-string > mozilla-b2g:master

Things are looking good now, a few comments on my change:

- I switched from using MozL10n.get to MozL10n.setAttributes followed by MozL10n.translateFragment which is required to force the synchronous translation of the node. Note that we should get rid of all others instances of MozL10n.get which is deprecated.

- I didn't use the FontSizeManager for adapting the string but rather I used the FontSizeUtils directly. The FontSizeManager didn't have the required functionality for the current layout, we might re-evaluate this when we change the layout.

- I've removed the CSS changes on the <span> element because they messed up the vertical alignment of certain languages. Instead I hard-coded the maximum size of the object in the code. This isn't pretty but it works well and we'll change it when we change the layout.

- I use the same resized font for both the 'callEnded' string and the duration counter. This is to prevent the duration counter from looking a lot bigger than the font in locales where we have to use a small font.

- The font size ranges I choose start with 16 points (the font size of the 'via SIMn') element below and 27 points (the per-existing maximum size). I think this is coherent with the rest of the design.

- Unit-tests were updated.

Comment 12

[Gabriele Svelto [:gsvelto]]

Attachment: UI Review screenshot

Here's a screenshot of my change in the Tamil, Russian, Malay, English, Italian, Bosnian and Arabic locales. Note that the duration and string use the same font size and they're always correctly aligned (previously alignment was wrong in certain scripts). See comment 11 for a full description of my change.

Comment 13

[Eric Pang [:epang] UX]

Comment on attachment 8597240 [details]
UI Review screenshot

Thanks for flagging me Gabriele.  The languages in the screens look good to me! R+

Comment 14

[Tamara Hills [:thills]]

Comment on attachment 8595372 [details] [review]
[gaia] gabrielesvelto:bug-1155901-resize-call-ended-string > mozilla-b2g:master

Hi Gabriele,

Thanks for all the explanations.  That was helpful!  However, I see a problem when there are multiple calls.  When you have two calls up, the call ended string seems to be in a much larger font and bleeds off the screen.  I will attach a screenshot.

Thanks,

-tamara

Comment 15

[Tamara Hills [:thills]]

Attachment: callendedmulticall.png

Here's the screenshot of multicall call ended.

Comment 16

[Gabriele Svelto [:gsvelto]]

(In reply to Tamara Hills [:thills] from comment #15)
> Here's the screenshot of multicall call ended.

Ouch, nice catch. The size of the font depends on the presence of the big-duration class in the parent calls container so I'll have to deal with that in this scenario.

Comment 17

[Gabriele Svelto [:gsvelto]]

I've just pushed another patch that takes care of the multi-call scenario. I'm not asking for review just yet because I've just realized that the string issue also affects group calls but then call group code has a different code path for this so it needs a dedicated fixup.

Comment 20

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8595372 [details] [review]
[gaia] gabrielesvelto:bug-1155901-resize-call-ended-string > mozilla-b2g:master

Obsoleting since I've created a new branch for this.

Comment 21

[Gabriele Svelto [:gsvelto]]

Attachment: [PULL REQUEST] Dynamically resize the 'call ended' string whenever it's translated

Review at your own leisure since this is not urgent. Here's my latest approach to fixing this problem as suggested by Zibi:

- I've replace the legacy mozL10n.get() call with a data-l10n-id for the callEnded string, this makes updating the contents of the element asynchronous.

- Because of this asynchronicity I've installed a MutationObserver which will check the string and whenever it's translated will re-size the font appropriately. Note that this works even when switching languages which - while unlikely - makes this solution very robust.

- To apply the new font size to the string I was facing the problem of having to apply a different font size depending on where the string was being used. Distinguishing between the single-call, multi-call and statusbar cases in JS was fragile and complicated so I've used CSS instead. What I did was to generate a dynamic CSS rule that applies the font-size appropriately using the same selectors we have in oncall.css and oncall_status_bar.css. This approach is also quite robust as it doesn't depend on the JS code and will always work even when switching views.

Comment 22

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8614640 [details] [review]
[PULL REQUEST] Dynamically resize the 'call ended' string whenever it's translated

lgtm

Comment 23

[Tamara Hills [:thills]]

Comment on attachment 8614640 [details] [review]
[PULL REQUEST] Dynamically resize the 'call ended' string whenever it's translated

Hi Gabriele,

Apologies for my lateness on this.

It looks good and I tested out some.  Do you think it's worthwhile to add test to make sure the MutationObserver gets called, disconnected, and observed again properly?

Thanks,
-tamara

Comment 24

[Gabriele Svelto [:gsvelto]]

(In reply to Tamara Hills [:thills] from comment #23)
> It looks good and I tested out some.  Do you think it's worthwhile to add
> test to make sure the MutationObserver gets called, disconnected, and
> observed again properly?

Yes, good point, I'll attach an updated patch with added tests.

Comment 25

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8614640 [details] [review]
[PULL REQUEST] Dynamically resize the 'call ended' string whenever it's translated

I've pushed a patch on top of the PR with a couple of unit-tests and one in particular which checks the order of events. I've also renamed the observer method from mutationObserver to observeMutation. Without realizing it I had given the same name to both the function and the field that would hold the MutationObserver object. It still worked but was quite confusing.

Comment 27

[Johan Lorenzo [:jlorenzo]]

Comms triage: Bug 1154634 was considered to be a blocker. Carrying over the flag.

Comment 28

[Tamara Hills [:thills]]

Comment on attachment 8614640 [details] [review]
[PULL REQUEST] Dynamically resize the 'call ended' string whenever it's translated

Hi Gabriele,

It looks good to me.  Thanks for adding the extra tests.  Sorry for the delay.

-tamara

Comment 29

[Gabriele Svelto [:gsvelto]]

Thanks for the review! Merged in gaia/master bc1d19af4d9d338c7cea23dd126c52814176db9f

https://github.com/mozilla-b2g/gaia/commit/bc1d19af4d9d338c7cea23dd126c52814176db9f

Comment 30

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed on the latest Nightly Flame 2.5 build and the latest Aries 2.5 Spark builds.

Actual Results:  The call ended text is not truncated in Malay, Tamil, or Bosnian.

Environmental Variables:
Device: Flame 2.5
BuildID: 20150716010206
Gaia: 981c61cdeb527fac8f8383c110df0e749eff67ea
Gecko: 72835344333f
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 42.0a1 (2.5) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Environmental Variables:
Device: Aries 2.5
BuildID: 20150716194428
Gaia: 77bc0d940bde2a5d2d4dfadfcccc6d8d77456d36
Gecko: 8d262d1d0ae5
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0