If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[V1.4][Dialer]The "Emergency number" is displayed as "Emergency nu..."

VERIFIED FIXED in Firefox OS v1.4

Status

Firefox OS
Gaia::Dialer
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: panda67231, Assigned: gtorodelvalle)

Tracking

(Blocks: 1 bug)

unspecified
2.0 S5 (4july)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 unaffected, b2g-v2.1 unaffected)

Details

(Whiteboard: [bamboo] [planned-sprint])

Attachments

(3 attachments, 7 obsolete attachments)

(Reporter)

Description

3 years ago
Created attachment 8430475 [details]
attachment.rar

[1.Description]:
The "Emergency number" is displayed as "Emergency nu..." and the text colour of "Connecting" and "via SIM1" is close to the background.

Attach the screenshots: EmergencyCall.png
Attach the logs: bugreport_EmergencyCall.txt & logcat_EmergencyCall.txt

Happened time: 4:00PM

[2.Testing Steps]: 
1. Open "dialer" app. 
2. Click on the phone pad icon on the bottom-right. 
3. MO an Emergency call (for example: 112)

[3.Expected Result]: 
3. The "Emergency number" should be displayed completely and the colour of the words "Connecting" and "via SIM1" should not be close to the background.

[4.Actual Result]: 
3. The "Emergency number" is displayed as "Emergency nu..." and the colour of the words "Connecting" and "via SIM1" is close to the background.

[5.Reproduction build]: 
Gaia        7709936aeb21859d1607dbd038489493803bb085
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/5bf038fae0f1
BuildID    20140522160202
Version    30.0

[6.Reproduction Frequency]: Always Recurrence
Blocks: 1015220
There's really two bugs here - so let's focus on the emergency call screen not displaying emergency number in full.

Beatriz - If the call screen displays "Emergency nu...", would this end up being a problem in certification?
Flags: needinfo?(brg)
Sorry, I need to know how it looks like in Spanish to say if it will be a cert blocker or not. Could you please attach the screen in Spanish or let me know the string in Spanish?
Thanks!
Rationale:
- If it is something like "Llam. de emergencia" is ok
- If it is something like "Llamada de e..." is not ok.
Flags: needinfo?(brg)
Can you get a screenshot of this bug in Spanish?
Flags: needinfo?(panda67231)
Keywords: qawanted
(Reporter)

Comment 4

3 years ago
Created attachment 8432928 [details]
Spanish.png

Attached Spanish screenshot
Flags: needinfo?(panda67231)
(In reply to Beatriz Rodríguez [:brg] from comment #2)
> Sorry, I need to know how it looks like in Spanish to say if it will be a
> cert blocker or not. Could you please attach the screen in Spanish or let me
> know the string in Spanish?
> Thanks!
> Rationale:
> - If it is something like "Llam. de emergencia" is ok
> - If it is something like "Llamada de e..." is not ok.

Based on the screenshot I'm seeing, this hits your cert blocking criteria, right?
Flags: needinfo?(brg)
(In reply to Jason Smith [:jsmith] from comment #5) 
> Based on the screenshot I'm seeing, this hits your cert blocking criteria,
> right?

Yes. Thanks for asking!
Flags: needinfo?(brg)

Updated

3 years ago
blocking-b2g: --- → 1.4+
Keywords: qawanted
I'm pretty sure we already shipped 1.1 and 1.3 with this issue. It should be fixed in bug 1007709 but that won't be uplifted because it's tied to the visual refresh.

QAwanted to check if it reproduces on 1.3.
Keywords: qawanted
(In reply to Anthony Ricaud (:rik) from comment #7)
> I'm pretty sure we already shipped 1.1 and 1.3 with this issue. It should be
> fixed in bug 1007709 but that won't be uplifted because it's tied to the
> visual refresh.
> 
> QAwanted to check if it reproduces on 1.3.

For obvious reasons - we can't do that. And that's really bad if we've shipped with this.
Keywords: qawanted
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 9

3 years ago
This is likely an issue across the board of languages, http://transvision.mozfr.org/string/?entity=apps/communications/dialer/shared.properties:emergencyNumber&repo=gaia_1_4 shows a host of long strings.

Evan added this in bug 870187.
Could we fix this as part of the visual refresh and uplift it to 1.4?
This bug is invalid after the visual refresh changes due to the following:

1. In visual refresh there's a feature to make the size of the contact's name text (in this case "Emergency number") fluid, so the text will not be truncated. (https://bugzilla.mozilla.org/show_bug.cgi?id=1007709)

2. The background image is the user's wallpaper and there's a gradient overlay on top of the image to help the legibility of the "connecting" text.

Plus, this "emergency call" design was made up in implementation, it never went through a proper visual design process. So there's a new Emergency call design and it is implemented already:  https://bugzilla.mozilla.org/show_bug.cgi?id=993951

Thanks
NI to Germán to confirm that this fluid size feature is spread as well in this case.
Flags: needinfo?(gtorodelvalle)
Vicky, the issue here is that they want it to be fixed in v1.4 and we can not uplift these Visual Refresh patches to this version so it seems that a specific patch for this bug in 1.4 would be the only option, not sure if it's feasible. As you say, German will have to confirm
Assignee: nobody → gtorodelvalle
Flags: needinfo?(gtorodelvalle)
Target Milestone: --- → 2.0 S5 (4july)
Whiteboard: bamboo → [bamboo] [planned-sprint]
Ok, this screen has been made up in development, and that's why we're having this kind of issues. This screen should have the wallpaper background and say "Llamada de emergencia" or "emergency call", no need for that wired siren illustration.
Created attachment 8447954 [details]
emergency_call_english.png
Attachment #8447954 - Flags: ui-review?(vpg)
Created attachment 8447956 [details]
emergengy_call_french.png
Attachment #8447956 - Flags: ui-review?(vpg)
Created attachment 8447957 [details]
21180.html
Attachment #8447957 - Flags: review?(etienne)
BTW, Etienne, notice I prepared a patch to v1.4 to solve the issue since trying to cherry-pick some commit in master could be kind of risky :-)
Attachment #8447956 - Flags: ui-review?(vpg) → ui-review+
Attachment #8447954 - Flags: ui-review?(vpg) → ui-review+
Attachment #8447957 - Flags: review?(etienne)
Comment on attachment 8447957 [details]
21180.html

Hi Anthony, I confirmed that all the original changes were needed to get the desired behaviour. If we do not pass the second parameter of HandledCall.formatPhoneNumber() as falsy, the "Emergency number" text is truncated as "Emergency nu..." (or whatever) when the call changes its state (is connected or hung up, for example). Consequently, https://github.com/mozilla-b2g/gaia/pull/21180/files#diff-24c70b7d83da756ecd2b39d890d1bb63R145 is NOT enough to solve the issue as we discussed via IRC.

So I am back in favor of the original changes. Of course, we can comment any issue you consider appropriate ;) Thanks!
Attachment #8447957 - Flags: review?(anthony)
Although I already did it, I can ask Loli to heavily test the patch in the device if you feel more confortable this way :)

Updated

3 years ago
Summary: [Flame][V1.4][Dialer]The "Emergency number" is displayed as "Emergency nu..." and the text colour of "Connecting" and "via SIM1" is close to the background when you MO an Emergency call. → [V1.4][Dialer]The "Emergency number" is displayed as "Emergency nu..."
Comment on attachment 8447957 [details]
21180.html

Thanks for the explanation. I'm going to forward the review to Doug since he has fresher experience on font size changes.

The patch still contains the emergency background changes and that's not ok to land. We need the least amount of changes possible.
Attachment #8447957 - Flags: review?(anthony) → review?(drs+bugzilla)
Comment on attachment 8447957 [details]
21180.html

I find myself in agreement with Anthony that this is unnecessarily risky, but I have another proposed method to do this which I'm pretty sure would work and is fairly low-risk.

Instead of half-cherry-picking some work that happened in other bugs and didn't get uplifted, I'd rather special case the emergency call header text. We can write a CSS selector that includes the .emergency-active state and forces the contact/number text (in this case, "Emergency number") down to the minimum allowed font size by FSM. This way, we're not depending on risky run-time resizing code.
Attachment #8447957 - Flags: review?(drs+bugzilla) → review-
Addendum: I think we should consider bolding/changing the color of the text (perhaps to red) if we're going to shrink it. Please ask for ui-review and ux-review once you have a patch ready.
Hi guys! I am need-infoing Vicky here since she was the one who asked for the wallpaper setting for emergency calls.

Although I was self-confident with the proposed solution :p I am OK with Doug's suggestion.

Anyhow, waiting on you guys to come to an agreement regarding the wallpaper setting issue. Thanks!
Flags: needinfo?(vpg)
On the other hand, I started implementing Doug suggestion and it is not as straight-forward as it may seem :) The point is that whenever the call changes its state a call to https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/handled_call.js#L231 is made forcing the calculation of the font size of the text shown. Since true is passed as the second parameter, the maximum font size is set and the "Emergency num..." is shown.
So I could include another filter to only update the font size if the call is not an emergency one. In fact, I did and prepared a new PR. I will not set the previous one as obsolete until we decide the way to go ;)
Created attachment 8449297 [details]
21265.html
Attachment #8449297 - Flags: review?(drs+bugzilla)
(In reply to Doug Sherk (:drs) from comment #23)
> Addendum: I think we should consider bolding/changing the color of the text
> (perhaps to red) if we're going to shrink it. Please ask for ui-review and
> ux-review once you have a patch ready.

This should not happen.
Flags: needinfo?(vpg)
Created attachment 8449300 [details]
emergency_call_outgoing.png

In fact, I included the wallpaper as the background image as requested by Vicky in comment 14 ;)
Attachment #8449300 - Flags: ui-review?(vpg)
Created attachment 8449301 [details]
emergency_call_ongoing.png

In fact, I included the wallpaper as the background image as requested by Vicky in comment 14 ;)
Attachment #8449301 - Flags: ui-review?(vpg)
Created attachment 8449302 [details]
emergency_call_ended.png

In fact, I included the wallpaper as the background image as requested by Vicky in comment 14 ;)
Attachment #8449302 - Flags: ui-review?(vpg)
Comment on attachment 8449302 [details]
emergency_call_ended.png

Call ended should have the hang up icon next to the text, not the outgoing one
Attachment #8449302 - Flags: ui-review?(vpg) → ui-review-
Sorry, I was not specific enough about the issues this patch was fixing. Please, consider only that the "Emergency number" text is shown with no ellipsis since this is a blocker for 1.4. Would you reconsider the previous ui-review taking this into consideration? :)

If you consider the icon issue should also be a blocker, please ping me via Skype and we can discuss it with Mari Ángeles and open a new bug if appropriate ;-) Thanks!
Attachment #8449301 - Flags: ui-review?(vpg) → ui-review+
Attachment #8449300 - Flags: ui-review?(vpg) → ui-review+
Need-infoing Vicky regarding my comment 32.
Flags: needinfo?(vpg)
NOt a blocker but an issue.
Flags: needinfo?(vpg)
Yeah, the point it to focus in this bug on what it has been reported in its title and description :-p If you consider it appropriate, no problem to create a new bug about the icon issue for 1.4 :) Would you be so kind, Loli, to create this bug for us so you can also track it? Thank you very much! ;)
(In reply to Doug Sherk (:drs) from comment #22)
> Comment on attachment 8447957 [details]
> 21180.html
> Instead of half-cherry-picking some work that happened in other bugs and
> didn't get uplifted, I'd rather special case the emergency call header text.
> We can write a CSS selector that includes the .emergency-active state and
> forces the contact/number text (in this case, "Emergency number") down to
> the minimum allowed font size by FSM. This way, we're not depending on risky
> run-time resizing code.

Anthony, do you agree with this?
Status: NEW → ASSIGNED
Flags: needinfo?(anthony)
I like how simple it is. That means introducing an !important and that's bad but because this is for a branch, I'm fine with it.
Flags: needinfo?(anthony)
Comment on attachment 8449297 [details]
21265.html

Thanks for making the proposed changes. This is definitely a lot cleaner. I'm surprised that we don't need an !important as Anthony said, but I tested it and it works fine for me. I left some comments on the PR.

(In reply to Victoria Gerchinhoren [:vicky] from comment #34)
> NOt a blocker but an issue.

Germán, would you file a followup for this and then reflag Vicky for ui-review on attachment 8449302 [details]?
Attachment #8449297 - Flags: review?(drs+bugzilla) → review-
Ups, in fact need-infoing Loli regarding comment 36 ;) Thanks!
Flags: needinfo?(lolimartinezcr)
(In reply to Germán Toro del Valle from comment #39)
> Ups, in fact need-infoing Loli regarding comment 36 ;) Thanks!

Thanks German!

New bug created https://bugzilla.mozilla.org/show_bug.cgi?id=1034032
Flags: needinfo?(lolimartinezcr)
Comment on attachment 8449297 [details]
21265.html

Comments by Doug included ;) Thanks!
Attachment #8449297 - Flags: review- → review?(drs+bugzilla)
Comment on attachment 8449297 [details]
21265.html

I just realized a really bad problem that the test that you added uncovered. I thought that the call node was cleared every time a call is made/ended, but it turns out that it persists. There's no teardown logic for the .emergency-call class, so we'll have to add that.

I left a couple of extra comments in the PR. On the next revision, please make sure that the following works correctly:

1) A regular call is made and doesn't have the .emergency-call class.
2) An emergency call is made and does have the .emergency-call class.
3) An emergency call is made and does have the .emergency-call class. Then it's ended and a regular call is made, and it doesn't have the .emergency-call class.
Attachment #8449297 - Flags: review?(drs+bugzilla) → review-
Comment on attachment 8449297 [details]
21265.html

Hi Doug, I included your comments and moved the included tests to make it clearer to the reader ;)

Regarding the reusing of the call node, they are not reused since each new HandleCall clones its own call node (see https://github.com/mozilla-b2g/gaia/blob/master/apps/callscreen/js/handled_call.js#L30 ). The problem with the test case failing was that I reused (copy & pasted) some code and was using an emergency number in that test case so obviously the emergency-call class was set, as it had to be. My fault, sorry!

Although the tests are being check, I am going to ask for a new review right now ;)
Attachment #8449297 - Flags: review- → review?(drs+bugzilla)
Great! Travis and TBPL are fine with the patch :)
Comment on attachment 8449297 [details]
21265.html

This patch contains the emergency wallpaper changes that have not been agreed to land on 1.4. Please ask for review again when the patch is only fixing the truncation problem.
Attachment #8449297 - Flags: review?(drs+bugzilla)
Vicky, according to comment 21 and comment 45 by Anthony and as we just discussed in IRC, I am removing the background change from this patch. If you want the wallpaper background instead of the red ringing bell in v1.4, please file a bug and nominate it for v1.4 ;) Thanks!
Flags: needinfo?(vpg)
(In reply to Anthony Ricaud (:rik) from comment #45)
> Comment on attachment 8449297 [details]
> 21265.html
> 
> This patch contains the emergency wallpaper changes that have not been
> agreed to land on 1.4. Please ask for review again when the patch is only
> fixing the truncation problem.

Thanks, Anthony. I could swear I read you write that somewhere else, but when I looked for it I couldn't find it.
(In reply to Doug Sherk (:drs) from comment #47)
> Thanks, Anthony. I could swear I read you write that somewhere else, but
> when I looked for it I couldn't find it.
Comment 21 ;)
Created attachment 8450991 [details]
21398.html

Showing the wallpaper fix removed from the patch ;)
Attachment #8447957 - Attachment is obsolete: true
Attachment #8449297 - Attachment is obsolete: true
Attachment #8450991 - Flags: review?(drs+bugzilla)
Attachment #8447954 - Attachment is obsolete: true
Attachment #8447956 - Attachment is obsolete: true
Attachment #8449301 - Attachment is obsolete: true
Attachment #8449300 - Attachment is obsolete: true
Attachment #8449302 - Attachment is obsolete: true
Attachment #8450991 - Flags: review?(drs+bugzilla) → review+
Merged in v1.4: https://github.com/mozilla-b2g/gaia/commit/5c9e1e4131d3ac8915ed88b72bb66dc7d97be6a0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-b2g-v1.4: --- → fixed
Resolution: --- → FIXED
Tested and working
Hamachi
1.4
Gecko-a9ee8ae
Gaia-5c9e1e4
Status: RESOLVED → VERIFIED
Removing NI as there's nothing to answer here.
Status: VERIFIED → RESOLVED
Last Resolved: 3 years ago3 years ago
Flags: needinfo?(vpg)
(In reply to Victoria Gerchinhoren [:vicky] from comment #52)
> Removing NI as there's nothing to answer here.

Vicky i have verified this bug, for this reason i change status to  VERIFIED. Please if you think it should be other status, please tell me.

Tested and working
Hamachi
1.4
Gecko-a9ee8ae
Gaia-5c9e1e4
Status: RESOLVED → VERIFIED
status-b2g-v2.0: --- → unaffected
status-b2g-v2.1: --- → unaffected
You need to log in before you can comment on or make changes to this bug.