Handle "answered-elsewhere" on incoming calls for desktop on Loop

VERIFIED FIXED in Firefox 34

Status

Hello (Loop)
Client
P1
normal
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
mozilla36
Points:
1
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox33 unaffected, firefox34+ verified, firefox35+ verified, firefox36 verified)

Details

(Whiteboard: [landed in fx-team])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Bug 1080491 has added an "answered-elsewhere" reason to the "terminate" message from the websocket.

This is received when an incoming call is answered on a different device.

When a client receives it, it should simply close the conversation window.
(Assignee)

Updated

3 years ago
Depends on: 1088351
(Assignee)

Comment 1

3 years ago
Created attachment 8510656 [details] [diff] [review]
Handle "answered-elsewhere" on incoming calls for desktop on Loop.

This handles the answered-elsewhere state. Well, in fact I've convinced myself it is safe to treat any terminated call as either:

- cancelled; which closes the window (and happens before a call is accepted)
- failed; which shows "Something went wrong" (and happens after a call has been accepted, e.g. call setup failed)

The dev server has the server-side workings for answered-elsewhere, though note that I've just filed bug 1088351 on an issue in the server relating to this which gives the wrong display.
Attachment #8510656 - Flags: review?(dmose)
(Assignee)

Updated

3 years ago
Assignee: nobody → standard8
backlog: --- → Fx34+
Iteration: --- → 36.1
Points: --- → 1
Priority: -- → P1
Target Milestone: --- → mozilla36
(Assignee)

Updated

3 years ago
Attachment #8510656 - Flags: review?(dmose) → review?(nperriault)
Comment on attachment 8510656 [details] [diff] [review]
Handle "answered-elsewhere" on incoming calls for desktop on Loop.

Review of attachment 8510656 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, rs=me with comments made over IRC addressed, and those below if proved.

::: browser/components/loop/test/desktop-local/conversation_test.js
@@ +439,5 @@
>                      reason: "timeout"
>                    }, "alerting");
>  
>                    sinon.assert.calledOnce(navigator.mozLoop.stopAlerting);
>                    done();

I think we can get rid of using done() here (and everywhere else where this is unneeded in this file — not 100% sure though).
Attachment #8510656 - Flags: review?(nperriault) → review+
(Assignee)

Comment 3

3 years ago
(In reply to Nicolas Perriault (:NiKo`) — needinfo me if you need my attention from comment #2)
> I think we can get rid of using done() here (and everywhere else where this
> is unneeded in this file — not 100% sure though).

Unfortunately, the use of promises forces us into handling this async state. I've had issues before with promises and trying to avoid the async, and it doesn't work.

Hopefully it'll be better with flux, as we won't need to spread promises around so much - its currently a workaround for getting the model set up correctly, which would be much easier with the conversationStore.
(Assignee)

Comment 4

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/826a94d4f2c6
(Assignee)

Comment 5

3 years ago
For verification:

- Set up three desktop browsers, two signed into one FxA account, and the third into a different account
-- Until bug 1080491 is deployed, these should use the development environment.

- Make a direct call from the different account to the other first account
- On one of the two browsers, accept the call
- Observe the other browser

Expected results:

- Before bug 1080491 is fixed/deployed:
-- The other browser shows "something went wrong" in the title bar
- After bug 1080491 is fixed/deployed:
-- The window closes on the other browser.
(Assignee)

Updated

3 years ago
Whiteboard: [landed in fx-team]
Comment on attachment 8510656 [details] [diff] [review]
Handle "answered-elsewhere" on incoming calls for desktop on Loop.

Approval Request Comment
[Feature/regressing bug #]:Handling answering calls when the user is logged in on multiple devices
[User impact if declined]: Client will keep ringing even if the call is answered on another device
[Describe test coverage new/current, TBPL]: unit tests, manual verification
[Risks and why]: No risk outside of Hello , low risk to Hello -- without this, the user needs to manually "cancel" the call on the device in order to stop ringing.  This was found when the server was recently updated.
[String/UUID change made/needed]: No strings
Attachment #8510656 - Flags: approval-mozilla-beta?
Attachment #8510656 - Flags: approval-mozilla-aurora?
[Tracking Requested - why for this release]:
Bug fix needed for multiple device support (Server will be updated soon, prior to Hello release)
tracking-firefox34: --- → ?
status-firefox33: --- → unaffected
status-firefox34: --- → affected
status-firefox35: --- → affected
status-firefox36: --- → affected
tracking-firefox34: ? → +
tracking-firefox35: --- → +
https://hg.mozilla.org/mozilla-central/rev/826a94d4f2c6
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Paul, can you please verify this is fixed in the latest Nightly?
status-firefox36: affected → fixed
Flags: qe-verify+
Flags: needinfo?(paul.silaghi)
QA Contact: anthony.s.hughes → paul.silaghi
Flags: in-testsuite?
Forgot to hit submit...
Tested this quite a bit against the dev server with Nightly 10/25 Linux & windows; works as intended.
Comment on attachment 8510656 [details] [diff] [review]
Handle "answered-elsewhere" on incoming calls for desktop on Loop.

Aurora+
Attachment #8510656 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/84b30e8c65ea
status-firefox35: affected → fixed
Verified on Aurora nightly 10/26 Linux
Comment on attachment 8510656 [details] [diff] [review]
Handle "answered-elsewhere" on incoming calls for desktop on Loop.

Beta+
Attachment #8510656 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox35: fixed → verified
status-firefox36: fixed → verified
https://hg.mozilla.org/releases/mozilla-beta/rev/67d9122b8c98
status-firefox34: affected → fixed
(In reply to Mark Banner (:standard8) from comment #5)
> Expected results:
> 
> - Before bug 1080491 is fixed/deployed:
> -- The other browser shows "something went wrong" in the title bar
> - After bug 1080491 is fixed/deployed:
> -- The window closes on the other browser.
Using the dev-server, now I see:
-- The window closes on the other browser.
and after 5 sec "Something went wrong" in other 2 browsers
FF 34b4, 36.0a1 (2014-10-28) Win 7
Thoughts ?
Flags: needinfo?(paul.silaghi) → needinfo?(standard8)
(Assignee)

Comment 17

3 years ago
(In reply to Paul Silaghi, QA [:pauly] from comment #16)
> Using the dev-server, now I see:
> -- The window closes on the other browser.
> and after 5 sec "Something went wrong" in other 2 browsers
> FF 34b4, 36.0a1 (2014-10-28) Win 7
> Thoughts ?

Can you clarify which browsers are the "other 2"? Did you have 3 browsers logged into the FxA account that you were calling?
Flags: needinfo?(standard8) → needinfo?(paul.silaghi)
(In reply to Mark Banner (:standard8) from comment #17)
> (In reply to Paul Silaghi, QA [:pauly] from comment #16)
> > Using the dev-server, now I see:
> > -- The window closes on the other browser.
> > and after 5 sec "Something went wrong" in other 2 browsers
> > FF 34b4, 36.0a1 (2014-10-28) Win 7
> > Thoughts ?
> 
> Can you clarify which browsers are the "other 2"? Did you have 3 browsers
> logged into the FxA account that you were calling?

Also, Pauly -- which browsers were running which versions?
Never mind. I've hit again https://bugzilla.mozilla.org/show_bug.cgi?id=1000237#c27, because I tested on a single machine.
Everything's fine in FF 34b4.
Status: RESOLVED → VERIFIED
status-firefox34: fixed → verified
Flags: needinfo?(paul.silaghi)
(Assignee)

Comment 20

2 years ago
Clearing in-testsuite requests for features that are being removed from Hello as part of the user journey work in bug 1209713.
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.