991118 - Link Clicker UI: Going back doesn't end the call

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Mark Banner (:standard8)]

STR:

1) Receive a link
2) Click Start, wait for call to be established
3) Click back

Expected results:

Call is ended (and/or maybe a prompt for do I really want to end the call).

Actual results:

The Video is hidden, the call continues and the camera is still connected.

Comment 1

[Maire Reavy [:mreavy]]

Escalating the priority because this is a security concern.  Folks expect that pressing back arrow will end the call based on other apps in the market.

Niko, can you prioritize fixing this (unless Mark or Dan would prefer to take this)?  Thanks.

Comment 2

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: https://github.com/mozilla/loop-client/pull/22

Loop-client part of the patch.

This patch does far more than just fixing this issue, it refactors the current architecture heavily, basically moving the sdk session handling to the model, which helps carrying the session information along while navigating using the router.

Comment 3

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: https://github.com/adamroach/gecko-dev/pull/16

Desktop part of the patch.

Comment 4

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8407752 [details] [review]
https://github.com/mozilla/loop-client/pull/22

Adding ametaireau for feedback while Standard8 is on PTO and dmose on another timezone atm.

Comment 5

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Comment on attachment 8407762 [details] [review]
https://github.com/adamroach/gecko-dev/pull/16

Adding ametaireau for feedback while Standard8 is on PTO and dmose on another timezone atm.

Comment 6

[Alexis Metaireau (:alexis)]

Comment on attachment 8407752 [details] [review]
https://github.com/mozilla/loop-client/pull/22

I added a bunch of comments on the github pull request.

This looks generally good to me. However, I don't see any test where we try to reproduce the steps that were causing trouble. 

Is it something we don't currently do?

Comment 7

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

> However, I don't see any test where we try to reproduce the steps that were causing trouble. 

Indeed. Test added in https://github.com/mozilla/loop-client/commit/edd4d38f8509934a8c09c06617a2e732c9a8e0fc, part of submitted PR.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8407752 [details] [review]
https://github.com/mozilla/loop-client/pull/22

r=dmose once any comments are appropriately addressed.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8407762 [details] [review]
https://github.com/adamroach/gecko-dev/pull/16

r=dmose

Comment 10

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Landed https://github.com/mozilla/loop-client/pull/22: https://github.com/mozilla/loop-client/commit/91799af6823956c478d30e4cd628962868639016

Comment 11

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Landed https://github.com/adamroach/gecko-dev/pull/16: https://github.com/adamroach/gecko-dev/commit/b2ab894c16afa781f8f765c0f492f8333d949ec4

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 8407762 [details] [review]
https://github.com/adamroach/gecko-dev/pull/16

Clearing obsolete feedback request.

Comment 13

[Mark Banner (:standard8)]

https://hg.mozilla.org/projects/elm/rev/3721e5de0761
https://hg.mozilla.org/projects/elm/rev/b6a0a6138e87

Comment 14

[Mark Banner (:standard8)]

https://hg.mozilla.org/mozilla-central/rev/3721e5de0761
https://hg.mozilla.org/mozilla-central/rev/b6a0a6138e87

Comment 15

[u279076]

Looks like this landed with tests. Does this need QA testing?

Comment 16

[u279076]

I confirm that going back now ends a call across all supported branches.