Behavior different between resuming user-held call and automatically held calls.

RESOLVED FIXED in Firefox OS master

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: pgravel, Assigned: gsvelto)

Tracking

unspecified
2.2 S14 (12june)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.5+, b2g-master fixed)

Details

(Whiteboard: [caf priority: p2][CR 841020])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Bug 1094878 changed the behavior of how calls are resumed based on whether the call was held by the user or automatically held.

While not a bad idea, GCF test cases never make such a distinction and the call is always expected to be automatically resumed when the active call is hung-up. Additionally, other mobile OS do not have this sort of logic either, making FFOS the odd one out.

It feels like the proper fix for bug 1094878 would have been to simplify the gaia portion and simply show the call as no longer being held (as it is shown in the telephony call list), rather than implement even more logic to create a special case for a user-held call. Some of the recent changes made in the dialer to make gaia less stateful might already fix the issue reported by bug 1094878.
(Reporter)

Updated

3 years ago
Flags: needinfo?(gtorodelvalle)
Germán doesn't work on the project anymore, NI'ing Gabriele.

Comms triage: Changing the behavior at this stage of the release is risky. Except if it's a certification blocker for 2.2, 3.0 should contain the fix.
blocking-b2g: 2.2? → 3.0+
Flags: needinfo?(gsvelto)
(Assignee)

Comment 2

3 years ago
Yes, some of the recent fixes (which were also uplifted to v2.2) might have solved the original issue. I'll try reverting bug 1094878 and see what happens now and if we get desirable behavior.
Flags: needinfo?(gtorodelvalle)

Updated

3 years ago
Whiteboard: [CR 841020]

Updated

3 years ago
Whiteboard: [CR 841020] → [caf priority: p2][CR 841020]
Assignee: nobody → gsvelto
Bumping this back to 2.2? pending the outcome of the test in comment 2 -- if there's a simple fix based on one of the more recent patches then we'd still want to consider this for 2.2.
blocking-b2g: 3.0+ → 2.2?
Comms triage: Too risky to have a fix of that importance after Code complete. Moreover backing out 1094878 will introduce a real impact for an end-user.
blocking-b2g: 2.2? → 3.0+

Updated

3 years ago
No longer blocks: 1063044
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
Flags: needinfo?(gsvelto)
Created attachment 8613960 [details] [review]
[gaia] gabrielesvelto:bug-1166635-revert-bug-1094878 > mozilla-b2g:master
(Assignee)

Comment 6

3 years ago
Comment on attachment 8613960 [details] [review]
[gaia] gabrielesvelto:bug-1166635-revert-bug-1094878 > mozilla-b2g:master

This reverts bug 1094878, the code needed a few modifications since we didn't want to get rid of certain small changes (such as toggleOnHoldButton becoming setShowIsHeld) which we rely on in the following changes.

To the reporter, could you please check if this patch fixes the problem for you now that the hold button state transitions are already properly handled?
Flags: needinfo?(pgravel)
Attachment #8613960 - Flags: review?(drs)
Comment on attachment 8613960 [details] [review]
[gaia] gabrielesvelto:bug-1166635-revert-bug-1094878 > mozilla-b2g:master

Rubber stamp r+ after a quick glance, since this is just a back-out, and I have no problem with removing this functionality.
Attachment #8613960 - Flags: review?(drs) → review+
(Assignee)

Comment 8

3 years ago
Thanks Doug, I'm merging this manually, I've verified that the behavior after reverting the previous patch is what is described in comment 0 (i.e. the remaining call is resumed and the state of the hold button is consistent with this).
(Assignee)

Comment 9

3 years ago
Merged to gaia/master 80a9434ba6c9887489f01aac7fc95a807e736c59

https://github.com/mozilla-b2g/gaia/commit/80a9434ba6c9887489f01aac7fc95a807e736c59

Try run is here:

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=246704817733b93b6f59c5d442809e21a9319dbe

Leaving the NI to confirm that this fixes the problem.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

3 years ago
Looks good to me, thanks!
Flags: needinfo?(pgravel)
status-b2g-v2.5: --- → fixed
status-b2g-master: --- → fixed
Target Milestone: --- → 2.2 S14 (12june)
status-b2g-v2.5: fixed → ---
(Assignee)

Updated

3 years ago
Blocks: 1181593
You need to log in before you can comment on or make changes to this bug.