894232 - [Dialer] Disable 'Put call on hold' feature for Leo due to unexpected interruptions during a call

Status: RESOLVED FIXED

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

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As seen on bug 882056 the 'Put on Hold' feature is not stable enough for leo and can cause unexpected interruptions of the call. All that happens because the little icon right next to the phone number, and a partially working proximity sensor. As of now the screen will not be turned off when the phone is moved to the ears, which easily let us put the call onto hold. A lot of people have already noticed that and didn't know what happened because the audio was gone.

As discussed on bug 882056 we would like to see this feature turned off for Leo while work is ongoing to get a working solution for koi. 

This is not a dupe of bug 882056 because it will only care about the disabling of this feature for Leo.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Thank you Henrik for filing this bug. This has been a constant annoyance for me while using the phone in my daily activities. Often the person on the other end of the phone call thinks it was dropped, and it took me multiple lost phone calls until I realized that it was my phone (not the network) that was causing the issue.

Comment 2

[bhavana bajaj [:bajaj]]

needsinfo'ing Wilfred to help confirm if "put on hold" is a 1.1 or a 1.2 feature ? If its 1.1 can product weigh on what the next steps should be here fiven the issue with the functionality ?

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to bhavana bajaj [:bajaj] from comment #2)
> needsinfo'ing Wilfred to help confirm if "put on hold" is a 1.1 or a 1.2
> feature ? If its 1.1 can product weigh on what the next steps should be here
> fiven the issue with the functionality ?

It's present on my unagi/1.1.0/beta build that I've been using, so I would make a strong assumption that it's part of 1.1.

Comment 4

[Wilfred Mathanaraj [:WDM]]

I think the question is if we should back this off and work on it for v1.2 or should it stay as is in v1.1

Just confirming this with teams.

Comment 5

[Wilfred Mathanaraj [:WDM]]

I would not - this feature from v1.1 as it may have other risks associated with it. I would sugges tot leave it in and work on the improvements for v1.2.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Wilfred Mathanaraj [:WDM] from comment #5)
> I would not - this feature from v1.1 as it may have other risks associated
> with it. I would sugges tot leave it in and work on the improvements for
> v1.2.

I don't really understand this comment. Do you mean that it should not be set to leo-? It's in conflict with the latter which sounds like the feature should be kept. Can you please clarify?

Again, this is a totally annoying behavior which affects me during EACH call I accept when I do not manually switch off the screen before starting to talk! I don't think that the feature in that state is good enough to be taken for 1.1. But well, it depends on the hardware and if a proximity sensor is available.

Comment 7

[Wilfred Mathanaraj [:WDM]]

I suggest: We keep the current implementation of call on hold for v1.1 (as there are other risks involved in removing this feature - I am told this is not just a simple patch to back out) - we implement the new design for v1.2.

i.e.
Close Bug 894232 as not fixing
Add Bug 882056 to v1.2 backlog

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

It doesn't have to be a patch backout, it can also just be a simple patch that removes the event listener from the button.

As Henrik has stated, this is an extremely annoying bug and makes using the phone near-impossible for those not technical enough to understand why all of their calls "keep getting dropped".

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L667 just needs to be commented out and it should fix the issue.

Comment 10

[Alex Keybl [:akeybl]]

Product call is minus. If we have a proposed patch that keeps the hold functionality but prevents this specific bug, please follow up with Wilfred over email.

Comment 11

[Jason Smith [:jsmith]]

(In reply to Alex Keybl [:akeybl] from comment #10)
> Product call is minus. If we have a proposed patch that keeps the hold
> functionality but prevents this specific bug, please follow up with Wilfred
> over email.

Also doing a WONTFIX this as a result as well. The only option on the table is a workaround on bug 882056 if it exists that's low risk.

Comment 12

[Anthony Ricaud (:rik)]

The patch in bug 882056 looks low risk to me. It's a pretty self contained patch. It should be easy to uplift to 1.1 if it's ok to do it that late.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Anthony Ricaud (:rik) from comment #12)
> The patch in bug 882056 looks low risk to me. It's a pretty self contained
> patch. It should be easy to uplift to 1.1 if it's ok to do it that late.

Thank you for looking in to this. I am quite worried that if this is not fixed many users of 1.1 will incorrectly think that their calls are dropped at a higher rate than other phones due to an accidental and silent entering of "hold".

Comment 14

[Francis Djabri [:djabber]]

(In reply to Alex Keybl [:akeybl] from comment #10)
> Product call is minus. If we have a proposed patch that keeps the hold
> functionality but prevents this specific bug, please follow up with Wilfred
> over email.

As Jared suggests, simply removing the event listener from the button would be a patch that would still keep the Hold functionality on the device, but would prevent users from accidentally putting calls on hold from the dialer. For example, users would still be able to put a call on hold using a BT headset.

Comment 15

[Wilfred Mathanaraj [:WDM]]

If its a low risk patch and it removes the most common use cases for a user accidentally putting call on hold then i am fine to include this patch.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Reopening the bug because of the reconsideration given in comment 15.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Re-requesting leo blocking flag, so we don't loose this important bug for leo.

Comment 18

[Anthony Ricaud (:rik)]

I don't get it. Are you suggesting we remove the hold call feature for 1.1? Most of our users won't have a BT headset.

Also, do we have a bug filed for this flaky proximity sensors? We can do a lot of workarounds but in the end, this is the root cause.

Comment 19

[Stephany Wilkes]

As Francis pointed out in an email thread that raised this issue yesterday, the dialer was implemented against specification to allow the user to put a single call on hold, even though it provides the user with no value. The way it has been implemented means that many users will accidentally put calls on hold and have no idea how this happened, nor how to fix it (and, as Jared points out in comment 13, may well assume the call was dropped). This is a severe usability issue.

This is why we want to go with Jared's recommendation, which provides a simple way to fix this reasonably quickly. That is to remove the event listener from the button. 

This won't back out the call hold functionality, but will prevent users from accidentally putting a call on hold from the dialer. The user will still be able to put a call on hold using the BT headset, which we believe was the original use case that was meant to be solved. 

Per the specs, there is no benefit in letting the user put a single call on hold from Dialer, as the user can just as well mute the call. 

This is why we'd like to get the patch lifted from bug 882056.

Comment 20

[Stephany Wilkes]

We suspect 891748, 887261 and 875378 are bug reports that are actually caused by this issue. Linking them here in case we need to update them accordingly as well.

Comment 21

[Alex Keybl [:akeybl]]

Over to Jaws for resolution. Triage originally thought this was going to remove functionality, but it's clear that isn't the case.

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

I'm not sure if I did this correctly, but here is the pull request:
https://github.com/mozilla-b2g/gaia/pull/11133

Wilfred, will the pull request be enough? Or should I put the patch in this bug?

Comment 23

[Wilfred Mathanaraj [:WDM]]

I am not the expert on how to land the patch but i think it may help if we can see how this patch has changed the implementation - a short video may be helpful.

I am going to ask if Alex can help with landing this patch on 1.1.

Comment 24

[Anthony Ricaud (:rik)]

Stephany: Thanks for the clarification.

I'll take the review, test it and land it.

Comment 25

[Anthony Ricaud (:rik)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11133/files#attch-to-bugzilla

Pointer to Github pull-request

Comment 26

[Anthony Ricaud (:rik)]

Merged in v1.1.0hd https://github.com/mozilla-b2g/gaia/commit/d6abf011ba3314f77fc687e73b842a46be6f3716

I made a mistake while merging this PR. It should have been a PR against master, not v1.1.0hd. I'll do the cherry-picking to v1-train and master manually then.

Comment 27

[Anthony Ricaud (:rik)]

Merged in v1-train with https://github.com/mozilla-b2g/gaia/commit/ff6bc9658bedcf126670777dde146b563e6d2a02
Merged in master with https://github.com/mozilla-b2g/gaia/commit/6e60abd7e7e2736d81101c5427ceb17ed8fbc205

Thanks Jared for this patch!

Comment 28

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Hi Rik,

lint error:
> ----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/communications/dialer/js/oncall.js -----
> Line 670, E:0001: Extra space at end of line

Comment 29

[Anthony Ricaud (:rik)]

Yuren: Thanks for the heads-up, I feel bad by not running the linter :( Anyway, looks like 63099db1ee2eea02dd75a997cf5e8241a56edf0c fixed this already.

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Wonderful. Finally I can have phone calls without any interruption again. Thanks!