Enpei Reporter
	Description • 11 years ago

+++ This bug was initially created as a clone of Bug #915074 +++

Gaia:     922de46ad5e4a8b513b24233a7f3bc54b93e9612
Gecko:    f28b860ef7a9fb9e0960b2aabaee55a42d17633b
BuildID   20131018170959
Version   26.0a2

* Symptom:
The switch call button name is not fully displayed, it is "Switch ca.." not "Switch call".

* Reproduce Steps
1. Make a call to other device or MT call to DUT.
2. MT a second call to DUT.
3. Check the switch call screen icon.

* Actual results:
Name next to switch button is "Switch ca.."

* Expected results:
Name next to switch button is "Switch call"

	Francis Lee [:frlee]
	Comment 1 • 11 years ago

according to triage result, changed to koi+

	Kai-Chih Hu [:khu]
	Comment 2 • 11 years ago

(In reply to Francis Lee [:frlee] from comment #1)
> according to triage result, changed to koi+

May I know who is working on this? Thanks.

	Gabriele Svelto [:gsvelto] (PTO)
	Comment 3 • 11 years ago

(In reply to Kevin Hu [:khu] from comment #2)
> May I know who is working on this? Thanks.

I'm handling this, will post a fix shortly.

	Gabriele Svelto [:gsvelto] (PTO)
	Comment 4 • 11 years ago

Not having the device at hand it's hard to tell what would be the right font size. Enpei could you try out with this patch and check if this is enough to make the whole test visible?

	Kai-Chih Hu [:khu]
	Comment 5 • 11 years ago

(In reply to Gabriele Svelto [:gsvelto] from comment #3)
> (In reply to Kevin Hu [:khu] from comment #2)
> > May I know who is working on this? Thanks.
> 
> I'm handling this, will post a fix shortly.

Thank you, Gabriele.

	Enpei Reporter
	Comment 6 • 11 years ago

(In reply to Gabriele Svelto [:gsvelto] from comment #4)
> Created attachment 821015 [details] [diff] [review]
> [PATCH] Further reduce font size to display the switch calls message
> 
> Not having the device at hand it's hard to tell what would be the right font
> size. Enpei could you try out with this patch and check if this is enough to
> make the whole test visible?

I git am patch and build this gaia and still has the problem. Any information I can provide to you from this device that can ease the problem you don't have the device?
Gaia:     98f87136d322523ba2f9c742bf9c83f76a7fd63c

The issue is for wasabi for sure, but eventually our code will be for other types of resolution. So if this issue is fixed, will it be a logic rule that can fit for other devices?

	Gabriele Svelto [:gsvelto] (PTO)
	Comment 7 • 11 years ago

(In reply to Enpei from comment #6)
> I git am patch and build this gaia and still has the problem. Any
> information I can provide to you from this device that can ease the problem
> you don't have the device?

I don't have the wasabi unfortunately. I tried on the Peak which should have the same qHD resolution as the wasabi but I cannot reproduce the problem on that one. Since this is really a very small problem could you ask to some of the Gaia developers in Taiwan? They should be able to fix it easily with devices available.

	Enpei Reporter
	Comment 8 • 11 years ago

(In reply to Gabriele Svelto [:gsvelto] from comment #7)
> I don't have the wasabi unfortunately. I tried on the Peak which should have
> the same qHD resolution as the wasabi but I cannot reproduce the problem on
> that one. Since this is really a very small problem could you ask to some of
> the Gaia developers in Taiwan? They should be able to fix it easily with
> devices available.

Hi Ken, could you check if any Gaia team member can support on this bug fixing?

	Ken Chang[:kenkai]
	Comment 9 • 11 years ago

Hi Tim, I wonder if there is anyone being able to support us.

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 10 • 11 years ago

Steve, or Rex, do you have bandwidth for this? If not borrow me your wasabi and I will try this myself :)

	Steve Chung [:steveck]
	Comment 11 • 11 years ago

I can take it.

	Steve Chung [:steveck]
	Comment 12 • 11 years ago

Hi Etienne,
Since number node apply the inline fontsize style, font size won't be changed until we add !important to the css class. Do you feel ok with simply add !important for switch class here?
Another interesting thing is kfontsize is never declared in formatPhoneNumber[1], and formatPhoneNumber seems never be called without maxfont. Do you think I should remove the dead code within the patch or it's necessary to preserve the non-maxfont case(and figure out where did kfontsize come from)?
[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/handled_call.js#L267

	Steve Chung [:steveck]
	Comment 13 • 11 years ago

s/kfontsize/kfontstep, sorry for the typo.

	Etienne Segonzac (:etienne)
	Comment 14 • 11 years ago

Forwarding to Rik since the font size computation code is fresher in his mind.
Also, I trust he'll find a way to prevent the use of |!important| :)

	Steve Chung [:steveck]
	Comment 15 • 11 years ago

Hi Rik, I got some thought if we want to get rid of the |!important| in css. Since we applied inline style for number node font size everywhere, maybe we can just reuse getNextFontSize to set the font size dymanically for avoiding the truncate because fixed font size for different locale is still dangerous, wdyt?

	Stephany Wilkes
	Comment 16 • 11 years ago

Hi all - It looks like the fix for this will be in the CSS, but feel free to flag UX if you need a font size adjustment. We've been doing a lot of those.

	bhavana bajaj [:bajaj]
	Comment 17 • 11 years ago

:steveck, I've just poked :rik to help with NI request here, what is the eta we are looking at fixing this once we have that info ?

	Steve Chung [:steveck]
	Comment 18 • 11 years ago

(In reply to bhavana bajaj [:bajaj] from comment #17)
> :steveck, I've just poked :rik to help with NI request here, what is the eta
> we are looking at fixing this once we have that info ?

If we only need to resize the font for smaller size in css, that might not the best solution here, but it's the easiest way that cost less than 5 mins... If JS changes is involved in the fixing, it's hard to estimate the eta.

	Anthony Ricaud (:rik)
	Comment 19 • 11 years ago

Sorry for this late answer. I only went back to my regular tasks yesterday. Bhavana: If I'm not answering on IRC, I can miss pings. Please followup via email if it's really important.

When I last looked at this, I couldn't understand why we apply a dynamic font size for this part of the UI. I think it should only apply to the call screen when the keyboard view is up. Unless I'm missing a reason, I believe we should remove this dynamic font size thingy when the keyboard view is not up.

	Anthony Ricaud (:rik)
	Comment 20 • 11 years ago

Removing the dynamic sizing is probably a change too big for now. Here's a proposition to remove the dynamic sizing when we display the "Switch calls" thing. I haven't tested it though.

+++ b/apps/communications/dialer/js/handled_call.js
@@ -225,7 +225,7 @@ HandledCall.prototype.restoreAdditionalContactInfo =
 HandledCall.prototype.formatPhoneNumber =
   function hc_formatPhoneNumber(ellipsisSide, maxFontSize) {
     // In status bar mode, we want a fixed font-size
-    if (CallScreen.inStatusBarMode) {
+    if (CallScreen.inStatusBarMode || CallScreen.cdmaCallWaiting) {
       this.numberNode.style.fontSize = '';
       return;
     }

	Steve Chung [:steveck]
	Comment 21 • 11 years ago

(In reply to Anthony Ricaud (:rik) from comment #20)
> Removing the dynamic sizing is probably a change too big for now. Here's a
> proposition to remove the dynamic sizing when we display the "Switch calls"
> thing. I haven't tested it though.
> 
> +++ b/apps/communications/dialer/js/handled_call.js
> @@ -225,7 +225,7 @@ HandledCall.prototype.restoreAdditionalContactInfo =
>  HandledCall.prototype.formatPhoneNumber =
>    function hc_formatPhoneNumber(ellipsisSide, maxFontSize) {
>      // In status bar mode, we want a fixed font-size
> -    if (CallScreen.inStatusBarMode) {
> +    if (CallScreen.inStatusBarMode || CallScreen.cdmaCallWaiting) {
>        this.numberNode.style.fontSize = '';
>        return;
>      }

It seems not the right timing for clearing the fontSize styling  because formatPhoneNumber will not be triggered while call held. We need to find another proper place to clear/reset fontsize(if we don't want to refactor this now). Reassign to Rex since he might have free cycle for this issue.

	Anthony Ricaud (:rik)
	Comment 22 • 11 years ago

You can maybe trigger a formatPhoneNumber when the call is held?

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 23 • 11 years ago

Since we're not refactoring it, this is a quick fix for it.
(With some background color fixing in css)

Rik may you help review this simple patch?

	Anthony Ricaud (:rik)
	Comment 25 • 11 years ago

Comment on attachment 8338393 [details]
patch

Oh, way simpler :) Thanks!

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 26 • 11 years ago

Thanks for helping Rik! :)

merged on master:
https://github.com/mozilla-b2g/gaia/commit/77420f0568432a3ed236137e27caa8cd832eed8c

	John Ford [:jhford] CET/CEST Berlin Time
	Comment 27 • 11 years ago

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 77420f0568432a3ed236137e27caa8cd832eed8c
  <RESOLVE MERGE CONFLICTS>
  git commit

	KM Lee [:rexboy] (inactivate after Sep. 22) Assignee
	Comment 28 • 11 years ago

v1.2
https://github.com/mozilla-b2g/gaia/commit/3507154456fb9367478ae808b6d8c326d0f51da8