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

RESOLVED FIXED in Firefox OS v2.0


Firefox OS
3 years ago
3 years ago


(Reporter: gtorodelvalle, Assigned: gandalf)



2.0 S2 (23may)
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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



(2 attachments, 1 obsolete attachment)

Created attachment 8417325 [details]
[screenshot] callscreen_conference_call_participant_call_ended.png

  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.

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

  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.

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?
blocking-b2g: --- → 2.0?
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → affected
Whiteboard: regression
triage: it's a regression
blocking-b2g: 2.0? → 2.0+
I think this is a regression of bug 914414. It might be the same kind of fix as in bug 994417.
Target Milestone: --- → 2.0 S2 (23may)


3 years ago
Assignee: nobody → anthony


3 years ago
Keywords: regression
Whiteboard: regression


3 years ago
Blocks: 914414

Comment 4

3 years ago
Assignee: anthony → gandalf
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

3 years ago
Created attachment 8421952 [details] [diff] [review]
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

3 years ago
Germán, can you provide STR on how to reproduce this bug? (including, how to initialize conference call)
Flags: needinfo?(gtorodelvalle)
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.
Flags: needinfo?(gtorodelvalle)

Comment 9

3 years ago
Created attachment 8422677 [details] [review]
pull request

I was able to reproduce the bug and confirm that this patch fixes it.
Attachment #8421952 - Attachment is obsolete: true
Attachment #8422677 - Flags: review?(anthony)
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
Attachment #8422677 - Flags: review?(anthony) → feedback+

Comment 11

3 years ago
Comment on attachment 8422677 [details] [review]
pull request

New, updated patch, now with a test!
Review while it's hot!
Attachment #8422677 - Flags: review?(anthony)
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|.
Attachment #8422677 - Flags: review?(anthony) → review+

Comment 13

3 years ago
fixed and added linters locally. Thanks!

Patch: https://github.com/mozilla-b2g/gaia/commit/7c782a4281d727f7882cb94c934571583bf2da20
Merge: https://github.com/mozilla-b2g/gaia/commit/e74485a527caa3fa07b9401f7e72761a731bb06c
Last Resolved: 3 years ago
Resolution: --- → FIXED


3 years ago
status-b2g-v2.0: affected → fixed
Flags: in-testsuite?
Flags: in-testsuite? → in-qa-testsuite?(jlorenzo)
Depends on: 1086269
QA Whiteboard: [fxosqa-auto-backlog?]
Flags: in-qa-testsuite?(jlorenzo)
I'm plussing for backlog, but I'm concerned with the number of moving parts for an automation test.
QA Whiteboard: [fxosqa-auto-backlog?] → [fxosqa-auto-backlog+]
You need to log in before you can comment on or make changes to this bug.