972017 - Desktop client needs call initiation to Contacts for logged in FxA users

Status: VERIFIED FIXED

(Hello (Loop) :: Client, enhancement, P3)

Comment 1

[Romain Testard [:RT]]

If a contact holds multiple identities (e-mail addresses and phone numbers), these multiple identities are sent to the server which will try to connect identities which are registered and connected.
If multiple identities are registered they are all notified though push notifications and the first identity which answers or declines causes all other engaged identities to have their incoming calls cancelled (first who answers or declines wins).

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Multiple destinations seems quite separable from this piece, and notably lower priority than just placing a call to a contact at all, so I've split that out to bug 1015491.

Comment 3

[:shell escalante]

tagged with contacts, depends on contacts & FxA bug, removed priority as it's blocked on 2 fronts now.

Comment 4

[Maire Reavy [:mreavy]]

I don't think this is needed for the first release of FxA & contacts.

Comment 5

[Maire Reavy [:mreavy]]

I assume this user story is to support the choice of audio only versus audio plus video (which is the default).  The ability to initiate a video call for a signed in FxA desktop user is a different bug.

Comment 6

[Romain Testard [:RT]]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #4)
> I don't think this is needed for the first release of FxA & contacts.

I disagree. 55% of Skype calls are audio only despite the fact that users have the option to use Video.
I think a fair share of calls won't happen if we force the use of Video to users.
A lot of people don't want to show their video - I think this is necessary for GA.

Comment 7

[Romain Testard [:RT]]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #5)
> I assume this user story is to support the choice of audio only versus audio
> plus video (which is the default).  The ability to initiate a video call for
> a signed in FxA desktop user is a different bug.

This bug is for call initiation from the contact list (direct calling) using audio only or audio+video. I can't see any other bug related to the "initiation of a video call for a signed in FxA desktop user" so I suggest we break this bug down for "audio only and "audio+video" call initiation if it makes technical sense.

Comment 8

[Maire Reavy [:mreavy]]

We should file a follow up bug to support audio only calling for signed in users.

Comment 9

[Mark Banner (:standard8)]

Attachment: Part 1 - Add a new controller view for selecting between incoming and outgoing calls in the Loop Conversation window. Also, set up a bare-bones outgoing pending conversation view.

This sets up two controller views for the conversation window - an overall controller, which selects between outgoing and incoming, and then an outgoing conversation controller.

In addition, there's a bare-bones pending view for the outgoing conversation I've added. Its intended that the sub-views be stateless, but we'll have to see how pratical that is as we go.

For now, I've set up a hidden pref ("loop.outgoingemail") to work around bug 1072323. Setting this pref to an email address, and then "receiving" an incoming call, will trigger the window to open as if it was an outgoing call.

Once we fix the bug, we can remove the pref as appropraite.

Next step is to get some of the action/dispatcher/store flows in place in part 2. Hence only requesting feedback for now, in case part 2 changes the shape of part 1 a bit.

Comment 10

[Mark Banner (:standard8)]

Attachment: Part 1 - Add a new controller view for selecting between incoming and outgoing calls in the Loop Conversation window. Also, set up a bare-bones outgoing pending conversation view.

Updated part 1, fixes a couple of extra issues, and updated to latest master.

Comment 11

[Mark Banner (:standard8)]

Attachment: Part 2 - Set up actions and a dispatcher and start to handle obtaining call data for outgoing Loop calls from the desktop client.

This part is all about setting up a basic dispatcher, actions and flow, to handle the initial pending call display, getting the data from the server, and then connecting to the websocket.

I've also included a change of state to terminated (e.g. for timeouts/rejections), to demonstrate the flows there.

Still hoping that we can pair review these later today.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 8496734 [details] [diff] [review]
Part 1 - Add a new controller view for selecting between incoming and outgoing calls in the Loop Conversation window. Also, set up a bare-bones outgoing pending conversation view.

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

Notes from pair reviewing with Mike over Vidyo.

::: browser/components/loop/content/js/conversation.jsx
@@ +198,5 @@
>     * At the moment, it does more than that, these parts need refactoring out.
>     */
>    var IncomingConversationView = React.createClass({
>      propTypes: {
> +     client: React.PropTypes.instanceOf(loop.Client).isRequired,

indent typo

@@ +500,5 @@
> +                          .isRequired,
> +      sdk: React.PropTypes.object.isRequired,
> +
> +      // XXX New types for OutgoingConversationView
> +      conversationStore: React.PropTypes.instanceOf(loop.ConversationStore).isRequired

As this is the main store, lets go with calling the prop "store" for now, and keep it short.

@@ +509,5 @@
> +    },
> +
> +    render: function() {
> +      if (this.state.outgoing) {
> +        return (<loop.conversationViews.OutgoingConversationView

Consider making this a constant and dropping the namespace

::: browser/components/loop/content/js/conversationViews.jsx
@@ +61,5 @@
> +            <button className="btn btn-cancel">
> +              {mozL10n.get("initiate_call_cancel_button")}
> +            </button>
> +          <div className="fx-embedded-call-button-spacer"></div>
> +            </div>

indentation

::: browser/components/loop/content/shared/css/conversation.css
@@ +224,5 @@
>  }
>  
> +/* General Call (incoming or outgoing). */
> +
> +.call-action-group {

Move these back down to save blame

::: browser/components/loop/test/desktop-local/conversation_test.js
@@ +138,5 @@
> +      ccView = mountTestComponent();
> +
> +      TestUtils.findRenderedComponentWithType(ccView,
> +        loop.conversationViews.OutgoingConversationView);
> +

nit: unnecessary blank line

::: browser/components/loop/ui/index.html
@@ +26,5 @@
>        window.OTProperties.configURL = window.OTProperties.assetURL + 'js/dynamic_config.min.js';
>        window.OTProperties.cssURL = window.OTProperties.assetURL + 'css/ot.css';
>      </script>
>      <script src="../content/shared/libs/sdk.js"></script>
> +    <script src="../content/shared/libs/react-0.11.1-prod.js"></script>

Drop this change for now.

@@ +58,5 @@
>      </script>
>      <script src="../content/js/panel.js"></script>
>      <script src="../content/js/conversation.js"></script>
>      <script src="ui-showcase.js"></script>
> + </body>

nit: bad indent change

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 8496799 [details] [diff] [review]
Part 2 - Set up actions and a dispatcher and start to handle obtaining call data for outgoing Loop calls from the desktop client.

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

Comments from pair review with Mike over vidyo.

::: browser/components/loop/content/js/client.js
@@ +225,5 @@
> +     * @param {Array} calleeIds an array of emails and phone numbers.
> +     * @param {String} callType the type of call.
> +     * @param {Function} cb Callback(err, result)
> +     */
> +    requestOutgoingCallData: function(calleeIds, callType, cb) {

nit: rename to setupOutgoingCall

@@ +240,5 @@
> +
> +          var postData = JSON.parse(responseText);
> +
> +          // This throws if the data is invalid, in which case only the failure
> +          // telemetry will be recorded.

This comment shouldn't be here.

@@ +241,5 @@
> +          var postData = JSON.parse(responseText);
> +
> +          // This throws if the data is invalid, in which case only the failure
> +          // telemetry will be recorded.
> +          var outgoingCallData = this._validate(postData,

Needs a try/catch in case data is invalid

::: browser/components/loop/content/js/conversation.jsx
@@ +512,5 @@
> +      this.props.conversationStore.on("change:outgoing", this._onChange, this);
> +    },
> +
> +    _onChange: function() {
> +      this.setState(this.props.conversationStore.attributes);

Move this to an inline function.

::: browser/components/loop/content/js/conversationViews.jsx
@@ +100,5 @@
> +      this.props.conversationStore.on("change", this._onChange, this);
> +    },
> +
> +    _onChange: function() {
> +      this.setState(this.props.conversationStore.attributes);

Make inline.

::: browser/components/loop/content/shared/js/conversationStore.js
@@ +23,4 @@
>        calleeId: undefined,
> +      // The call type for the call.
> +      // XXX Don't hard-code, this comes from the data in bug 1072323
> +      callType: "audio-video",

Define constants for callTypes

@@ +122,5 @@
> +        callState: "gather"
> +      });
> +
> +      if (this.get("outgoing")) {
> +        this._getOutgoingCallData();

change to _setupOutgoingCall

@@ +176,5 @@
> +
> +      this._websocket.promiseConnect().then(
> +        function() {
> +          this.dispatcher.dispatch(new sharedActions.ConnectionProgress({
> +            state: "connecting"

Add note, this is the call state.

@@ +207,5 @@
> +        case "alerting":
> +          action = new sharedActions.ConnectionProgress({
> +            state: progressData.state
> +          });
> +      }

nit: add break and a default case (log it)

::: browser/components/loop/content/shared/js/dispatcher.js
@@ +29,5 @@
> +     * @param {Object} store The store object to register
> +     * @param {Array} eventTypes An array of action names
> +     */
> +    register: function(store, eventTypes) {
> +      eventTypes.forEach(function (type) {

nit: no space after function

::: browser/components/loop/test/desktop-local/client_test.js
@@ +275,5 @@
> +          { calleeId: calleeIds, callType: callType }
> +        );
> +      });
> +
> +      it("should call the callback if the request is successful", function() {

Need a test for invalid data - copy test for "should send an error if the data is not valid"

::: browser/components/loop/test/desktop-local/conversationViews_test.js
@@ +108,5 @@
> +    it("should update the rendered views when the state is changed.",
> +      function() {
> +        conversationStore.set({callState: "connecting"});
> +
> +        view = mountTestComponent();

Check pending conversation view is displayed for test clarity

::: browser/components/loop/test/shared/conversationStore_test.js
@@ +252,5 @@
> +
> +      it("should dispatch a connection failure action on failure", function(done) {
> +        rejectConnectPromise();
> +
> +        connectPromise.then(function() {}, function() {

Consider doing an assert in the success case, to be explicit about test failure.

::: browser/components/loop/test/shared/dispatcher_test.js
@@ +49,5 @@
> +      });
> +
> +      cancelAction = new sharedActions.CancelCall();
> +
> +      gather1 = {

Rename these to gatherStore1 etc

@@ +102,5 @@
> +      beforeEach(function() {
> +        // Restore the stub, so that we can easily add a function to be
> +        // returned. Unfortunately, sinon doesn't make this easy.
> +        cancel1.cancelCall.restore();
> +        sandbox.stub(cancel1, "cancelCall", function() {

Can we clean this up by replacing functions or some other means?

@@ +110,5 @@
> +          sinon.assert.notCalled(gather2.gatherCallData);
> +        });
> +      });
> +
> +      it("should not dispatch an action if the previous action hasn't finished", function() {

Add some notes to explain these tests better.

::: browser/components/loop/test/shared/validate_test.js
@@ +4,5 @@
> +/*global chai, validate */
> +
> +var expect = chai.expect;
> +
> +describe('Validator', function() {

nit: use double quotes

Comment 14

[Mark Banner (:standard8)]

Attachment: [checked in] Part 1 - Add a new controller view for selecting between incoming and outgoing calls in the Loop Conversation window. Also, set up a bare-bones outgoing pending conversation view.

Updated to address review comments. The only extra thing I change was to reset the title of the ui-showcase, as I noticed some of the views were changing it. To make this work, I had to pass a dummy websocket instance to one of the views to avoid exceptions, hopefully that will go away soon.

Comment 15

[Mark Banner (:standard8)]

Attachment: [checked in] Part 2 - Set up actions and a dispatcher and start to handle obtaining call data for outgoing Loop calls from the desktop client.

Updated per the review comments.

Comment 16

[Mike de Boer [:mikedeboer]]

Comment on attachment 8497471 [details] [diff] [review]
[checked in] Part 1 - Add a new controller view for selecting between incoming and outgoing calls in the Loop Conversation window. Also, set up a bare-bones outgoing pending conversation view.

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

Just like we discussed :) Thanks!

::: browser/components/loop/content/js/conversation.jsx
@@ +12,5 @@
>    "use strict";
>  
>    var sharedViews = loop.shared.views,
> +      sharedModels = loop.shared.models,
> +      OutgoingConversationView = loop.conversationViews.OutgoingConversationView;

nit: Since you're touching this, could you put each statement on its own line with a `var`?

Comment 17

[Mike de Boer [:mikedeboer]]

Comment on attachment 8497581 [details] [diff] [review]
[checked in] Part 2 - Set up actions and a dispatcher and start to handle obtaining call data for outgoing Loop calls from the desktop client.

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

\o/ This is going to be grand.

::: browser/components/loop/content/js/client.js
@@ +237,5 @@
> +            this._failureHandler(cb, err);
> +            return;
> +          }
> +
> +          var postData = JSON.parse(responseText);

JSON.parse can throw... I don't think hawkRequest() catches that, right?

Comment 18

[Mark Banner (:standard8)]

Pushed the first two parts with the final comments fixed:

https://hg.mozilla.org/integration/fx-team/rev/326b3f37e469
https://hg.mozilla.org/integration/fx-team/rev/5d2c69ebb321

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/326b3f37e469
https://hg.mozilla.org/mozilla-central/rev/5d2c69ebb321

Comment 20

[Mark Banner (:standard8)]

Attachment: Part 3 - Finish the view flow transition for direct calling for Loop.

I was going to hook up the sdk parts of this patch, but it seemed simpler just to get the entire view flow in place and then add in the sdk parts in the fourth part.

Hence this adds in flows for:

- transitioning to a vastly non-functional ongoing call view
- implementing hangup call going to the feedback view
- implementing retry and cancel buttons on the call failed view
- implementing cancel on the pending call view

The styles are wrong, I'm proposing we fix that later once we've got everything working (or alongside the next patch maybe).

Comment 21

[Mark Banner (:standard8)]

Attachment: [checked in] Part 3 - Finish the view flow transition for direct calling for Loop.

Updated patch based on direct pair review comments over vidyo.

Comment 22

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

Comment on attachment 8498777 [details] [diff] [review]
[checked in] Part 3 - Finish the view flow transition for direct calling for Loop.

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

Pair reviewed over Vidyo, some supplementary nits you may want to address before landing. Great work!

::: browser/components/loop/content/js/conversationViews.jsx
@@ +48,3 @@
>        callState: React.PropTypes.string,
>        calleeId: React.PropTypes.string,
> +      enableCancelButton: React.PropTypes.bool.isRequired

Nit: I'd rather see it being optional and have a default set through getDefaultProps()

@@ +281,5 @@
> +            dispatcher={this.props.dispatcher}
> +            callState={this.state.callState}
> +            calleeId={this.state.calleeId}
> +            enableCancelButton={this.state.callState !== CALL_STATES.INIT &&
> +                                this.state.callState != CALL_STATES.GATHER}

Why mixing !== and != here?

Also please move this test to a private method, eg. isCancellable() or equivalent.

::: browser/components/loop/content/shared/js/conversationStore.js
@@ +27,5 @@
> +    // One of the two parties has indicated successful media set up,
> +    // but the other has not yet.
> +    HALF_CONNECTED: "half-connected",
> +    // Both endpoints have reported successfully establishing media.
> +    CONNECTED: "connected"

While we're now prefixing call states with "cs-", how about prefixing these with "ws-" for consistency and killing any possible confusion?

@@ +241,5 @@
> +     * Retries a call
> +     */
> +    retryCall: function() {
> +      var callState = this.get("callState");
> +      if (!callState === CALL_STATES.TERMINATED) {

if (callState !== CALL_STATES.TERMINATED) is probably what we want :)

::: browser/components/loop/ui/ui-showcase.jsx
@@ +265,5 @@
>                  <StartConversationView conversation={mockConversationModel}
>                                         client={mockClient}
>                                         notifications={notifications} />
>                </div>
>              </Example>

Please note the UI showcase shows an error in the console as loop.Dispatcher is undefined; you need to fake it or to include the script into that page.

Comment 23

[Mark Banner (:standard8)]

(In reply to Nicolas Perriault (:NiKo`) — needinfo me if you need my attention from comment #22)
> > +    CONNECTED: "connected"
> 
> While we're now prefixing call states with "cs-", how about prefixing these
> with "ws-" for consistency and killing any possible confusion?

I didn't do this as currently these map directly to the websocket states, as received from the websocket. To change these would mean we'd need to be tweaking them in various places as they are received.

> Please note the UI showcase shows an error in the console as loop.Dispatcher
> is undefined; you need to fake it or to include the script into that page.

Fixed by making the showcase not run the init functions for the panel and conversation files.

Comment 24

[Mark Banner (:standard8)]

Comment on attachment 8498777 [details] [diff] [review]
[checked in] Part 3 - Finish the view flow transition for direct calling for Loop.

https://hg.mozilla.org/integration/fx-team/rev/248b7b6c41d5

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/248b7b6c41d5

Comment 26

[Mark Banner (:standard8)]

Attachment: Part 4 - Hook up the OT sdk to the direct calling window for Loop.

This completes everything necessary to hook up the sdk and have the call completely working.

There's still issues like styles and other things, but I'll file follow-up bugs for those.

Comment 27

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

Comment on attachment 8499614 [details] [diff] [review]
Part 4 - Hook up the OT sdk to the direct calling window for Loop.

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

This looks great. r=me conditional to comments being addressed before landing, nits are optional.

::: browser/components/loop/content/js/conversationViews.jsx
@@ +143,5 @@
>      },
>  
> +    // height set to 100%" to fix video layout on Google Chrome
> +    // @see https://bugzilla.mozilla.org/show_bug.cgi?id=1020445
> +    publisherConfig: {

Nit: Please create a getPublisherConfig method, see below.

@@ +162,5 @@
>        };
>      },
>  
> +    componentWillMount: function() {
> +      this.publisherConfig.publishVideo = this.props.video.enabled;

Nit: This could be avoided by creating a getPublisherConfig() method, keep reading.

@@ +179,5 @@
> +      // The SDK needs to know about the configuration and the elements to use
> +      // for display. So the best way seems to pass the information here - ideally
> +      // the sdk wouldn't need to know this, but we can't change that.
> +      this.props.dispatcher.dispatch(new sharedActions.SetupStreamElements({
> +        publisherConfig: this.publisherConfig,

Nit: Here we could call the getPublisherConfig() method mentioned previously, which could easily return a config object with a `publishVideo` property matching the current `video.enabled` prop value.

@@ +207,2 @@
>      updateVideoContainer: function() {
>        var localStreamParent = document.querySelector('.local .OT_publisher');

document? that should be this.getDOMNode().querySelector(".local")

BTW that means we could probably reuse the _getElement method here.

::: browser/components/loop/content/shared/js/actions.js
@@ +111,5 @@
> +     */
> +    MediaConnected: Action.define("mediaConnected", {
> +    }),
> +
> +    SetMute: Action.define("setMute", {

Missing jsdocs.

::: browser/components/loop/content/shared/js/conversationStore.js
@@ +176,3 @@
>          case WS_STATES.HALF_CONNECTED:
>          case WS_STATES.CONNECTED: {
> +          // Nothing to do.

Nit: Ensuring that callState is set to CALL_STATES.ONGOING is probably safe here, and shouldn't trigger a component rerender if shouldComponentUpdate is properly implemented.

::: browser/components/loop/content/shared/js/otSdkDriver.js
@@ +206,5 @@
> +    /**
> +     * Publishes the local stream if the session is connected
> +     * and the publisher is ready.
> +     */
> +    _maybePublishLocalStream: function() {

Nit: This could be a monad, we're ready for haskell :p

@@ +213,5 @@
> +        this.session.publish(this.publisher);
> +
> +        // Now record the fact, and check if we've got all media yet.
> +        this._publishedLocalStream = true;
> +        this._checkAllStreamsConnected();

Nit: I'd rather dispatch the action from here, but would keep the test in a meaningfully named method:

if (this._checkAllStreamsConnected()) {
  this.dispatcher.dispatch(new sharedActions.MediaConnected());
}

::: browser/components/loop/test/desktop-local/conversationViews_test.js
@@ +187,5 @@
> +        var hangupBtn = view.getDOMNode().querySelector('.btn-hangup');
> +
> +        React.addons.TestUtils.Simulate.click(hangupBtn);
> +
> +        sinon.assert.called(dispatcher.dispatch);

Nit: Any specific reason we're not using calledOnce here? If so, sinon.assert.calledWithMatch implicitely test for called() already, so this could be removed.

::: browser/components/loop/test/shared/otSdkDriver_test.js
@@ +15,5 @@
> +  beforeEach(function() {
> +    sandbox = sinon.sandbox.create();
> +
> +    fakeElement1 = {fake: 1};
> +    fakeElement2 = {fake: 2};

For clarity, these should be renamed fakeLocalElement and fakeRemoteElement respectively.

Comment 28

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/fx-team/rev/b28e496d535b

Comment 29

[Mark Banner (:standard8)]

Although this has landed, it isn't accessible until bug 1072323 is fixed.

However, there is currently a way of testing this without bug 1072323. It would be good to start getting some advance testing on the flow. 

1) Start up two nightlies, sign into loop via FxA on each, with different accounts.
2) On one of the nightlies 'A', set the pref "loop.outgoingemail" to the account email of the other nightly.
3) On nightly 'A' with the pref set, copy a call url from the panel.
4) Open that in nightly 'B' browser, and start the call

=> At this stage an outgoing call will appear on nightly 'A', and start a call, so that 'nightly' B receives an incoming call.

Cancelling the call url (standalone UI) won't do anything. In this state, you've effectively initiated the outgoing call.

With outgoing calls, you can:

- Accept the call
- Connect the call and have a conversation
- Give feedback at both ends when the conversation is finished
- Be notified if the call is rejected/fails for some reason (with a generic failure message)
- Retry the call if it fails

Most of these are also covered in bugs that this block, that I'll be marking as fixed.

Comment 30

[u279076]

My current priority is making sure all the ducks are lined up for Firefox 34 moving to Beta in roughly a week. Paul, can you do some testing of this in the latest Nightly builds based on comment 29?

Comment 31

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/mozilla-central/rev/b28e496d535b

Comment 32

[Mark Banner (:standard8)]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #30)
> My current priority is making sure all the ducks are lined up for Firefox 34
> moving to Beta in roughly a week.

FYI We're expecting this to be uplifted before the merges.

> Paul, can you do some testing of this in
> the latest Nightly builds based on comment 29?

Note that a test case I forgot would be to log in on Firefox Nightly, and then direct call to the Firefox OS application. I tested this on Friday before around the time that this landed.

Comment 33

[Paul Silaghi, QA [:pauly]]

(In reply to Mark Banner (:standard8) from comment #29)
> However, there is currently a way of testing this without bug 1072323. It
> would be good to start getting some advance testing on the flow.
Possible issues:
1. 'retry/cancel' buttons are smaller than 'cancel/accept' buttons from the incoming call window
2. bug 1078261 - not sure if related

Comment 34

[Mark Banner (:standard8)]

(In reply to Paul Silaghi, QA [:pauly] from comment #33)
> (In reply to Mark Banner (:standard8) from comment #29)
> > However, there is currently a way of testing this without bug 1072323. It
> > would be good to start getting some advance testing on the flow.
> Possible issues:
> 1. 'retry/cancel' buttons are smaller than 'cancel/accept' buttons from the
> incoming call window

Bug 1077710

> 2. bug 1078261 - not sure if related

I suspect that's a separate issue (I just commented there).

Comment 35

[Paul Silaghi, QA [:pauly]]

Thanks Mark.
Marking as verified fixed FF 35.0a1 (2014-10-05).

Comment 36

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8497471 [details] [diff] [review]
[checked in] Part 1 - Add a new controller view for selecting between incoming and outgoing calls in the Loop Conversation window. Also, set up a bare-bones outgoing pending conversation view.

Approval Request Comment
Part of the staged Loop aurora second uplift set

Comment 37

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a29ef7c8a3e0
https://hg.mozilla.org/releases/mozilla-aurora/rev/fc8fca136da3
https://hg.mozilla.org/releases/mozilla-aurora/rev/4fda0b154861
https://hg.mozilla.org/releases/mozilla-aurora/rev/669ecc39ceae

Comment 38

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8497471 [details] [diff] [review]
[checked in] Part 1 - Add a new controller view for selecting between incoming and outgoing calls in the Loop Conversation window. Also, set up a bare-bones outgoing pending conversation view.

Approval previously granted to jesup to land this change as part of the second batch of Loop fixes to land on Aurora. Marking the approval after the fact.

Comment 39

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 34b1 Win 7