Description: Phone numbers in the Group Call list are truncated. Repro Steps: 1) Update Buri to Build ID: 20140218004003 2) Call (or be called by) multiple numbers that are not on the device's contact list. 3) Tap the merge button to create a conference call. 4) Tap the > icon in the group call header to being up the list of group call participants. 5) Observe the appearance of the participant phone numbers. Actual: The phone numbers appear truncated. Expected: The entire phone number is displayed. Environmental Variables Device: Buri 1.3 mozilla RIL Build ID: 20140218004003 Gecko: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/b5afe0b10e93 Gaia: df60e9b49207d64da5647ab15760c122adabfba4 Platform Version: 28.0 Firmware Version: V1.2-device.cfg Notes: Repro frequency: 100% See attached screenshots

Does not repro on Buri 1.2 mozilla RIL. The entire number for the group call participants are shown. Build ID: 20140213004001 Gecko: https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/4fd893896d02 Gaia: 539a25e1887b902b8b25038c547048e691bd97f6 Platform Version: 26.0 Firmware Version: V1.2-device.cfg

Well that's definitely a wrong UX. We can't do much with phone numbers if they only show 7 digits.

triage: 1.3+ regresion

This issue started reproducing on the 10/29/13 1.3 build. - Works - Device: Buri v1.3 MOZ RIL BuildID: 20131028040200 Gaia: 31c260b5c853fe78d6bf44c6aeec52a651c3f025 Gecko: 59ff3a2a708a Version: 27.0a1 Firmware Version: V1.2-device.cfg - Broken - BuildID: 20131029040205 Gaia: 92c097fd7ac9886f937d9decb0e03ab673deaa1b Gecko: 518f5bff0ae4 Version: 28.0a1 Firmware Version: V1.2-device.cfg

I am unable to reproduce on my keon with latest 1.3 build from geeksphone site. Build ID: 20140220010226 Can we verify that this is still happening?

This issue does reproduce for me on the 02/20/14 1.3 build using a Buri device. Device: Buri v1.3 MOZ RIL BuildID: 20140220004003 Gaia: a43904d9646109b48836d62f7aa51e499fbf4b2e Gecko: 32fd5d798477 Version: 28.0 Firmware Version: V1.2-device.cfg

I just put v1.3 BuildID 20140220004003 on a Buri, flashed Gaia a43904d9646109b48836d62f7aa51e499fbf4b2e manually, and was unable to repro. I then hacked Gaia to add an additional 3 digits, so that the page was displaying 13 total digits for each number, and no ellipses showed up. I also tried adding 5 calls at once, like in the screenshot, but still no dice. Matthew, how many digits are in the phone numbers from your screenshot? Is there anything else about your build that might be different from the one I downloaded from pvt (that you know of)?

(In reply to Michael Henretty [:mhenretty] from comment #7) > I just put v1.3 BuildID 20140220004003 on a Buri, flashed Gaia > a43904d9646109b48836d62f7aa51e499fbf4b2e manually, and was unable to repro. > I then hacked Gaia to add an additional 3 digits, so that the page was > displaying 13 total digits for each number, and no ellipses showed up. I > also tried adding 5 calls at once, like in the screenshot, but still no dice. > > Matthew, how many digits are in the phone numbers from your screenshot? Is > there anything else about your build that might be different from the one I > downloaded from pvt (that you know of)? Hey Michael, That is strange that you aren't seeing what we are seeing here. We just follow the STR and each number after the first one dialed in the group call list is truncated. The builds we are pulling for 1.3 (b2g28) are from the nightly section of the PVT website. I am not sure what could be different about them unfortunately. :(

Wait, the first one is not truncated? Does this happen with only two phone calls? How many digits do these phone numbers contain?

After speaking with Matthew Vaughan on IRC, I came up with an updated STR: 1) Update Buri to Build ID: 20140218004003 2) Initiate a call (call out or call in). 3) After call is connected, CALL OUT to another phone (calling in will not repro). 4) Tap the > icon in the group call header to being up the list of group call participants. 5) Observe the appearance of the participant phone numbers. An important detail is that the initial call will not have the ellipses, but all proceeded calls (that were called out, not in) will indeed have the ellipses. This is actually intended behavior, otherwise the "Connecting" message will overlap the phone number. The problem is that when we move to group call, we no longer have this "Connecting" and so we need to recalculate where/if to place the ellipses.

Right now, ellipses are added programmatically when adding a call to a call list. We do this by examining the size of the contact number container, then we fill a fake number container with the contact number and incrementally remove characters until the width of the fake container is less than the real container. We then transfer the contents of the fake container to the real container. There are a few problems that I've found here that I would like to address: 1.) After we add ellipses for an initial call, and upon answering a second call, the size of the first call number container shrinks (because we add the merge icon, and so increase the padding), but we never update the position of the ellipses to reflect this shrinkage. This can cause text overlap. 2.) When receiving a second call, we attempt to add ellipses to its number container while its parent is hidden. This makes weird things happen like division by 0 here: https://github.com/mozilla-b2g/gaia/blob/18667e43291a313288a099c25ff8fb455ea67cc0/apps/communications/dialer/js/utils.js#L118. In the end, since everything has a 0 width when the parent is hidden, we add no ellipses for the second incoming call no matter how many characters it contains. 3.) When merging calls into a group call, we take the existing ellipses position, which doesn't make a lot of sense since we have a lot more room to work with on the group call screen. We should recalculate the ellipses position when merging to a group. 4.) Ellipses calculations in general don't take into account the font weight of the number container, which can changed based on the situation. 5.) We have text-overflow: ellipses set on this number container (which it actually falls back to when we try and calculate ellipses with a hidden container). Given that it appears to work well, we should just use css for this when we are adding ellipses to the end of the container.

Hi Anthony, Would you take a look at this dialer patch? I tried to strike a balance between fixing the issues mentioned in comment 11, and introducing as little risk of regression as possible. The biggest thing I'm not sure about is that we no longer force maxFontSize for newly answered calls. I think this makes the call list more consistent, since we were reformatting without maxFontSize on the oncall window resize event (which seems to happen whenever calls are dropped or grouped). Also, I would love to drop the `addEllipses('end')` functionality in favor of `text-overflow: ellipses`, but I'm not sure of all the repercussions. In any case, I'm open to alternative approaches. Just let me know!

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call Related bugs: bug 974320 and bug 944340. I think a better fix here is to remove any dynamic style.fontSize. As you said, text-overflow will do a better work. If a call has no contact information attached, we shouldn't try to dynamically resize it. Does that make sense? Also, I have no idea why we passed "true" to formatPhoneNumber. This sounds wrong, could you do a little git archaeology to find out why?

(In reply to Anthony Ricaud (:rik) from comment #13) > Also, I have no idea why we passed "true" to formatPhoneNumber. This sounds > wrong, could you do a little git archaeology to find out why? Passing "true" for maxFontSize started in bug 827753. Moreover, bug 827753 comment 2 seems to suggest that this was not the best way forward, but it is unclear looking at the bug or github how this problem was resolved to francisco's satisfaction. In any case, UX doesn't want dynamic font sizing in this call list (944340 comment 2), so this "true" param seems to be correct ultimately. > I think a better fix here is to remove any dynamic style.fontSize. As you > said, text-overflow will do a better work. If a call has no contact > information attached, we shouldn't try to dynamically resize it. Passing "true" for maxFontSize is what removes dynamic font sizing. The problem is that when the resize event gets fired on the window object, we no longer pass true to the format function [1] (which I believe is the cause of bug 974320). Also, from bug 944340 comment 2 it sounds like we never want to use dynamic font sizing in the call list. Personally, I like dynamic font sizing here because without it anytime you have a phone number or contact over 7 characters long you are going to get an ellipses. I'd rather see more of the contact name/number even if it does mean multiple font sizes in the list. But I defer to UX on this, and will submit a patch that forces maxFontSize always in the call list. 1.) https://github.com/mozilla-b2g/gaia/blob/3826f33d046e6d9f701faf8ea62a6fcb5d21e675/apps/communications/dialer/js/calls_handler.js#L398

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call Ok, I have disabled dynamic font sizing for the call list. Note, I have two commits in there so you can see the interdiff, but I will squash before merging. Also, this should also fix bug 974320 and bug 944340, but I will not mark duplicate until we can land this and verify those are fixed separately (ie. don't want to block on them).

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call Since this is 1.3+, adding the other dialer peers to see who has the most bandwidth for review.

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call See github. Just a few simple comments :) Let's file a follow up to have a separate dom node where we apply inline font-size changes in order to separate more clearly the places where we want it and the place where we don't.

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call Ok, I addressed and responded to you comments on github. Also, I had a small question in there. Let me know your thoughts :) (In reply to Etienne Segonzac (:etienne) from comment #17) > Let's file a follow up to have a separate dom node where we apply inline > font-size changes in order to separate more clearly the places where we want > it and the place where we don't. Can you explain a little more what you are looking for here?

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call Arrg, should have seen this earlier: Numbers on the main call screen display are truncated way to early in the scenarios where you have 2 open lines (2 call, one held for example)/ Probably because the width of the number node is not fixed properly in css so viewWidth is too small here [1]. (In reply to Michael Henretty [:mhenretty] from comment #18) > Ok, I addressed and responded to you comments on github. Also, I had a small > question in there. Let me know your thoughts :) On github? Didn't saw it... :/ > > (In reply to Etienne Segonzac (:etienne) from comment #17) > > Let's file a follow up to have a separate dom node where we apply inline > > font-size changes in order to separate more clearly the places where we want > > it and the place where we don't. > > Can you explain a little more what you are looking for here? HandledCalls own their dom, and we display them in different contexts. We have a few of those contexts where we want to display the number/name with a specific style while bypassing all the crazy auto-font-sizing-and-ellipsis stuff. We could offer that (a node that will *not* have inline styling and always contain the full number/name). [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/utils.js#L110

(In reply to Etienne Segonzac (:etienne) from comment #19) > Arrg, should have seen this earlier: > Numbers on the main call screen display are truncated way to early in the > scenarios where you have 2 open lines (2 call, one held for example)/ > > Probably because the width of the number node is not fixed properly in css > so viewWidth is too small here [1]. Right, when a second call is added to the main call list, we increase the padding of all numberContainers, presumably to give enough room for the merge icon [1]. This added padding gives us less space to work with when calculating the ellipses position. However, the reason you see the truncated phone numbers in my patch but not on master is this: adding the second call causes the resize event to fire on the window, and on master this causes the call list to be reformatted with auto-font sizing enabled [2]. So for short phone numbers, the auto font sizing allows us to not have ellipses. But when the name/number is sufficiently long, this resize code causes bug 974320 (because ellipsesSide defaults to "begin" when not passed in to formatPhoneNumber) [3]. For an illustration of this, I've attached screenshot taken from current master [4]. Here the number view is highlighted in red, the fake number view is in green. Notice that the first number has a smaller font then the second. Also notice that the fake number container has some left padding. This padding also gives us less space to work with, but if we remove it the second phone number overlaps with the "Connecting" message. In any case, my patch ensures that we are always using maxFontSize, and always placing ellipses at the end for the call lists items. We do lose some digits in the phone number, but as the screenshot illustrates, this was already happening in master for the second call in the call list. Perhaps we could gain some digits here by decreasing the maxFontSize for number containers in this call list? Let me know what you think is best. 1.) https://github.com/mozilla-b2g/gaia/blob/ecb5f5f725d67d8b6d658178ac65885febf26f02/apps/communications/dialer/style/oncall.css#L431 2.) https://github.com/mozilla-b2g/gaia/blob/3826f33d046e6d9f701faf8ea62a6fcb5d21e675/apps/communications/dialer/js/calls_handler.js#L398 3.) https://bug974320.bugzilla.mozilla.org/attachment.cgi?id=8378191 4.) https://bug974211.bugzilla.mozilla.org/attachment.cgi?id=8383366 > (In reply to Michael Henretty [:mhenretty] from comment #18) > > Ok, I addressed and responded to you comments on github. Also, I had a small > > question in there. Let me know your thoughts :) > > On github? Didn't saw it... :/ Unfortunately, after updating my PR, github hides all my responses as "outdated". In any case, here is my question: https://github.com/mozilla-b2g/gaia/pull/16543#discussion_r10103991 > > (In reply to Etienne Segonzac (:etienne) from comment #17) > > > Let's file a follow up to have a separate dom node where we apply inline > > > font-size changes in order to separate more clearly the places where we want > > > it and the place where we don't. > > > > Can you explain a little more what you are looking for here? > > HandledCalls own their dom, and we display them in different contexts. > We have a few of those contexts where we want to display the number/name > with a specific style while bypassing all the crazy > auto-font-sizing-and-ellipsis stuff. We could offer that (a node that will > *not* have inline styling and always contain the full number/name). Gotcha. Would bug 944340 be a good place to handle these changes? My two cents (taken with a grain of salt given my limited knowledge of the dialer): I think re-using the same DOM node for call list, group call list, and status bar is ok. But we shouldn't be calling formatPhoneNumber for any of these number containers. I think dynamic font sizing is only useful when actually dialing in a new number (ie keypad entry), and ellipses should be handled by text-overflow rather than programmatically using a hidden dom element. I didn't want to do that for this patch because it seemed risky for 1.3, and I don't know the dialer too well. Sorry for the long response, but given the async nature of this I wanted to include all the info I could. It's too bad that we live so far apart, because I feel like we could flesh this out with a 5 minute conversation. Maybe I should move to Paris... :) Actually though, I am going to try and ping you on IRC tonight before I go to bed.

Thanks so much for digging through all of this Mike. I think we're close. * For the truncation issue, how about just adding a `padding: 0;` in the `.fake-number` rule in oncall.css? I does the job without having side effects (and we're in side effects hell already here :)) * I'd like to rename `updateSingleLine()` to `updateCallsDisplay()` in call_screen.js and call_screen_test.js And that's it :)

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call all good, r=me with a green travis :)

master: https://github.com/mozilla-b2g/gaia/commit/c42af9efb7a5e7e1e85373b7d342a3ea088b32cf

Please request approval-gaia-v1.3 on this patch for uplift.

Comment on attachment 8380143 [details] Github PR, Reformat call list with each new/dropped call NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. Fixes for dialer truncation [User impact] if declined: hard to see in-progress phone numbers [Testing completed]: yes. [Risk to taking this patch] (and alternatives if risky): in-progress list display

continued: [Testing completed]: Tested manually on the device (buri) by initiating incoming calls, then either calling out, or having a second call come in. Verified that will both 10 digit phone numbers, and with long contact names, we are able to see all the characters that are able to fit into their respective containers. Also, added unit tests to make sure certain dialer actions (updating the call list displays) behave the way we expect.

v1.3: 0ca7a557d99dc56eb93de7a111264c87811019d4

Tested and working Device: Hamachi 1.3 Build ID:20140304091708 Platform version 28.0 Gecko: 21d6a90 Gaia: 242e477 1.4 Build ID: 20140304082349 Platform version 30.0a1 Gecko: ffe5883 Gaia:71f78f7

Hi all, I test with the patch and find it brings another problem, that is ,the number/contact name overlaps with the string "Connecting" when the second call coming. PLease see the pictures in attachment 8388358 [details]. Can you tell me your result? Thanks.

(In reply to Lingling Zhao from comment #32) > Hi all, > > I test with the patch and find it brings another problem, that is ,the > number/contact name overlaps with the string "Connecting" when the second > call coming. > > PLease see the pictures in attachment 8388358 [details]. > > Can you tell me your result? > > Thanks. That's known - see bug 979624 which is tracking fixing that issue.