1033196 - When dialing the user name part which containing j/g characters would be incomplete

Status: RESOLVED FIXED

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

Description

[ben.song]

Attachment: screenshot_when_dialing.jpg

Comment 1

[ben.song]

for the DOM node of username in communications/dialer/oncall.html containing styles:font-size,height,line-height,those do not fit each other,so that part of the content of the node would be out of the bottom-border.

Comment 2

[ben.song]

Attachment: 1033196.patch

I make a patch,In that the style of line-height be lower so that the content of the node will move up and all of the content will be in the view area.

Comment 3

[helloarvin]

Hi etienne,

Would you please help review the patch attachment 8449229 [details] [diff] [review] or give us some feedback?

Thanks a lot.

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 8449229 [details] [diff] [review]
1033196.patch

Review of attachment 8449229 [details] [diff] [review]:
-----------------------------------------------------------------

Redirecting to Rik, please use the review flag when asking for review, it helps us get to it quicker.

Comment 5

[Anthony Ricaud (:rik)]

This is a 1.4 only issue. Before considering this patch, we need to know if we want to take this in 1.4.  Can you ask blocking-b2g: 1.4?

Comment 6

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Ni? Ivan to triage as this is 1.4 specific issue.
It's already by visual refresh in 2.0.

Comment 7

[Ivan Tsay (:ITsay)]

Take this visual refresh in v1.4

Comment 8

[Anthony Ricaud (:rik)]

Attachment: ui-review-1033196.png

Vicky, Carol: Does moving those contact names a bit higher is ok with you? (Broadcasting the ui-review a bit since it's a 1.4 blocker)

Comment 9

[Anthony Ricaud (:rik)]

Attachment: ui-review-1033196.png

I totally messed up the previous screenshot, didn't I? :)

Comment 10

[Anthony Ricaud (:rik)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/21640

Comment 11

[Carol Huang [:Carol]]

Comment on attachment 8454712 [details]
ui-review-1033196.png

Hi Anthony,
The UI adjustment looks good!!!Thanks!

Comment 12

[Anthony Ricaud (:rik)]

Comment on attachment 8454720 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/21640

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): I don't know, maybe it has always been like this?
[User impact] if declined: Some letters would not be complete
[Testing completed]: Just by developing on a flame.
[Risk to taking this patch] (and alternatives if risky): Very easy to revert if it proves to cause issues.
[String changes made]: None

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8454720 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/21640

This bug is 1.4+ and so doesn't need any other approval to land.

Comment 14

[Victoria Gerchinhoren [:vicky]]

Comment on attachment 8454712 [details]
ui-review-1033196.png

PLease set UI review to the new Dialer owner. Carol Huang <[e-mail]>

Comment 15

[Anthony Ricaud (:rik)]

Comment on attachment 8454712 [details]
ui-review-1033196.png

Vicky: I did :) And she already answered. So good to go.

Comment 16

[Anthony Ricaud (:rik)]

Merged in https://github.com/mozilla-b2g/gaia/commit/7e49e66ae1f8f48bb23a4a551a8e393b0ae8a228.

Comment 17

[Yeojin Chung [:YeojinC]]

New test case needs to be added. There is no existing test case.

Comment 18

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

New test case written in moztrap to cover descenders:

https://moztrap.mozilla.org/manage/case/14232/

Comment 19

[helloarvin]

Hi Anthony,

Could you please help merge the patch into v1.3t, thank you?

Comment 20

[bhavana bajaj [:bajaj]]

I've emailed wayne to help with this request, also leaving a NI here.

Comment 21

[helloarvin]

(In reply to bhavana bajaj [:bajaj] from comment #20)
> I've emailed wayne to help with this request, also leaving a NI here.

Thank you all very much.

Comment 22

[Wayne Chang [:wchang]]

Hi Arvin,

We're only landing critical fixes into 1.3t now, this won't be included. If you need this, can you patch it at your end?

Thanks

Comment 23

[helloarvin]

OK, I'll patch it locally.

To be honest, the issue does belong to a common issue especially for our customers. So please wayne help consider my request once more.

Thank you.