1005862 - [Call Screen][Conference call] Wrong participant leaving the conference notification message

Status: RESOLVED FIXED

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

Description

[Germán Toro del Valle (:gtorodelvalle)]

Attachment: [screenshot] callscreen_conference_call_participant_call_ended.png

STR:
  1. Establish a conference call with at least 2 additional parties. At least one of them has to be a contact from the contact list. 
  2. Hang up the call in the device of the contact.

EXPECTED:
  3. The text "NameOfTheContact has left the call." is shown where NameOfTheContact is the name of the contact :)

OBSERVED:
  3. The text "{{ caller }} has left the call." is shown. Please the attached screenshot.

It works as expected when the participant who leaves the conference is a phone number with no associated contact in the contacts list.

Comment 1

[Noemí Freire (:noemi)]

Hi,

This issue is not occurring on today's (5/5) v1.4 build:
Device: Hamachi
BuildId: 20140505063510
Gecko: 3ed3bbf
Gaia: fccb15d
Platform version: 30.0

Under this scenario, "The "NameOfTheContact has left the call" message is properly shown so adding regression keyword and nominating to 2.0?

Comment 2

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

triage: it's a regression

Comment 3

[Anthony Ricaud (:rik)]

I think this is a regression of bug 914414. It might be the same kind of fix as in bug 994417.

Comment 4

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

taking

Comment 5

[Anthony Ricaud (:rik)]

From IRC: "<gandalf> Rik: oh, didn't ask you. Are you ok with me taking over bug 1005862". Yup, I haven't started on it yet. Thanks!

Comment 6

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

Attachment: maybe a fix?

Ok, so first of all, I don't know how to establish a conference call in Gaia. I only got two 1-1 calls and was able to switch between them.

Second of all, I'm really concerned about the amount of multi-type variables in callscreen code. It makes it really hard to debug.

My guess is that in one case self._cachedInfo is in fact an Object, when it gets data from getPhoneNumberPrimaryInfo in line 178 (and line 187), but the function itself may return multiple data types.

I also verified that all other uses of mozL10n.get in callscreen/js and it seems that there are no other type mismatches.

Comment 7

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

Germán, can you provide STR on how to reproduce this bug? (including, how to initialize conference call)

Comment 8

[Anthony Ricaud (:rik)]

To merge calls, you can press the "merge arrow" on the right of one of the calls. If it doesn't work, you should see the message "Unable to merge calls" at the bottom of the screen.

Comment 9

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

Attachment: pull request

I was able to reproduce the bug and confirm that this patch fixes it.

Comment 10

[Anthony Ricaud (:rik)]

Comment on attachment 8422677 [details] [review]
pull request

Yup, proper fix. We need a new test inside this suite though: https://github.com/mozilla-b2g/gaia/blob/d364c473012b7731688fe0ef4bca92358cee18c5/apps/callscreen/test/unit/handled_call_test.js#L441

Comment 11

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

Comment on attachment 8422677 [details] [review]
pull request

New, updated patch, now with a test!
Review while it's hot!

Comment 12

[Anthony Ricaud (:rik)]

Comment on attachment 8422677 [details] [review]
pull request

Good change but the linter is not happy, see Travis.

You should install jshint and gjslint to have the pre-commit hook do the linting for you. You should have a link to the instructions to do that in your terminal when you do |git commit|.

Comment 13

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

fixed and added linters locally. Thanks!

Patch: https://github.com/mozilla-b2g/gaia/commit/7c782a4281d727f7882cb94c934571583bf2da20
Merge: https://github.com/mozilla-b2g/gaia/commit/e74485a527caa3fa07b9401f7e72761a731bb06c

Comment 14

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

I'm plussing for backlog, but I'm concerned with the number of moving parts for an automation test.