Bug 1081023 - Handle call url changes to the format for Loop's call links. r=nperriault a=loop-only
authorMark Banner <standard8@mozilla.com>
Thu, 23 Oct 2014 09:42:21 +0100
changeset 233766 7203a9983dfc1584899b3947116ddf693f4b9c98
parent 233765 5d0f7645977b10b342c6ed60069fe9db5955e572
child 233767 3dc36b88d62338996cc0b24b40f7d62ef52efd54
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnperriault, loop-only
bugs1081023
milestone35.0a2
Bug 1081023 - Handle call url changes to the format for Loop's call links. r=nperriault a=loop-only
browser/components/loop/content/js/conversation.js
browser/components/loop/content/js/conversation.jsx
browser/components/loop/content/shared/js/conversationStore.js
browser/components/loop/content/shared/js/utils.js
browser/components/loop/standalone/content/js/webapp.js
browser/components/loop/standalone/content/js/webapp.jsx
browser/components/loop/standalone/server.js
browser/components/loop/test/desktop-local/conversation_test.js
browser/components/loop/test/shared/conversationStore_test.js
browser/components/loop/test/standalone/webapp_test.js
browser/components/loop/ui/index.html
--- a/browser/components/loop/content/js/conversation.js
+++ b/browser/components/loop/content/js/conversation.js
@@ -617,17 +617,17 @@ loop.conversation = (function(mozL10n) {
     // we transition across (bug 1072323).
     var conversation = new sharedModels.ConversationModel(
       {},                // Model attributes
       {sdk: window.OT}   // Model dependencies
     );
 
     // Obtain the callId and pass it through
     var helper = new loop.shared.utils.Helper();
-    var locationHash = helper.locationHash();
+    var locationHash = helper.locationData().hash;
     var callId;
     var outgoing;
     var localRoomStore;
 
     // XXX removeMe, along with noisy comment at the beginning of
     // conversation_test.js "when locationHash begins with #room".
     if (navigator.mozLoop.getLoopBoolPref("test.alwaysUseRooms")) {
       locationHash = "#room/32";
--- a/browser/components/loop/content/js/conversation.jsx
+++ b/browser/components/loop/content/js/conversation.jsx
@@ -617,17 +617,17 @@ loop.conversation = (function(mozL10n) {
     // we transition across (bug 1072323).
     var conversation = new sharedModels.ConversationModel(
       {},                // Model attributes
       {sdk: window.OT}   // Model dependencies
     );
 
     // Obtain the callId and pass it through
     var helper = new loop.shared.utils.Helper();
-    var locationHash = helper.locationHash();
+    var locationHash = helper.locationData().hash;
     var callId;
     var outgoing;
     var localRoomStore;
 
     // XXX removeMe, along with noisy comment at the beginning of
     // conversation_test.js "when locationHash begins with #room".
     if (navigator.mozLoop.getLoopBoolPref("test.alwaysUseRooms")) {
       locationHash = "#room/32";
--- a/browser/components/loop/content/shared/js/conversationStore.js
+++ b/browser/components/loop/content/shared/js/conversationStore.js
@@ -404,17 +404,17 @@ loop.store.ConversationStore = (function
 
         // Now close the websocket.
         this._websocket.close();
         delete this._websocket;
       }
 
       // XXX: The internal callId is different from
       // this.get("callId"), see bug 1084228 for more info.
-      var locationHash = new loop.shared.utils.Helper().locationHash();
+      var locationHash = new loop.shared.utils.Helper().locationData().hash;
       var callId = locationHash.match(/\#outgoing\/(.*)/)[1];
       navigator.mozLoop.releaseCallData(callId);
     },
 
     /**
      * Used to handle any progressed received from the websocket. This will
      * dispatch new actions so that the data can be handled appropriately.
      */
--- a/browser/components/loop/content/shared/js/utils.js
+++ b/browser/components/loop/content/shared/js/utils.js
@@ -86,18 +86,25 @@ loop.shared.utils = (function(mozL10n) {
       //     so we need a better check. Bug 1065403.
       return !!window.MozActivity && /mobi/i.test(platform);
     },
 
     isIOS: function(platform) {
       return this._iOSRegex.test(platform);
     },
 
-    locationHash: function() {
-      return window.location.hash;
+    /**
+     * Helper to allow getting some of the location data in a way that's compatible
+     * with stubbing for unit tests.
+     */
+    locationData: function() {
+      return {
+        hash: window.location.hash,
+        pathname: window.location.pathname
+      };
     }
   };
 
   /**
    * Generates and opens a mailto: url with call URL information prefilled.
    * Note: This only works for Desktop.
    *
    * @param  {String} callUrl   The call URL.
--- a/browser/components/loop/standalone/content/js/webapp.js
+++ b/browser/components/loop/standalone/content/js/webapp.js
@@ -896,20 +896,33 @@ loop.webapp = (function($, _, OT, mozL10
 
     var feedbackApiClient = new loop.FeedbackAPIClient(
       loop.config.feedbackApiUrl, {
         product: loop.config.feedbackProductName,
         user_agent: navigator.userAgent,
         url: document.location.origin
       });
 
-    // Obtain the loopToken and pass it to the conversation
-    var locationHash = helper.locationHash();
-    if (locationHash) {
-      conversation.set("loopToken", locationHash.match(/\#call\/(.*)/)[1]);
+    // Obtain the loopToken
+
+    var match;
+
+    // locationHash supports the old format urls.
+    var locationData = helper.locationData();
+    if (locationData.hash) {
+      match = locationData.hash.match(/\#call\/(.*)/);
+    } else if (locationData.pathname) {
+      // Otherwise, we're expecting a url such as /c/<token> for calls.
+      match = locationData.pathname.match(/\/c\/([\w\-]+)/);
+    }
+    // XXX Supporting '/\/([\w\-]+)/' is for rooms which are to be implemented
+    // in bug 1074701.
+
+    if (match && match[1]) {
+      conversation.set({loopToken: match[1]});
     }
 
     React.renderComponent(WebappRootView({
       client: client, 
       conversation: conversation, 
       helper: helper, 
       notifications: notifications, 
       sdk: OT, 
--- a/browser/components/loop/standalone/content/js/webapp.jsx
+++ b/browser/components/loop/standalone/content/js/webapp.jsx
@@ -896,20 +896,33 @@ loop.webapp = (function($, _, OT, mozL10
 
     var feedbackApiClient = new loop.FeedbackAPIClient(
       loop.config.feedbackApiUrl, {
         product: loop.config.feedbackProductName,
         user_agent: navigator.userAgent,
         url: document.location.origin
       });
 
-    // Obtain the loopToken and pass it to the conversation
-    var locationHash = helper.locationHash();
-    if (locationHash) {
-      conversation.set("loopToken", locationHash.match(/\#call\/(.*)/)[1]);
+    // Obtain the loopToken
+
+    var match;
+
+    // locationHash supports the old format urls.
+    var locationData = helper.locationData();
+    if (locationData.hash) {
+      match = locationData.hash.match(/\#call\/(.*)/);
+    } else if (locationData.pathname) {
+      // Otherwise, we're expecting a url such as /c/<token> for calls.
+      match = locationData.pathname.match(/\/c\/([\w\-]+)/);
+    }
+    // XXX Supporting '/\/([\w\-]+)/' is for rooms which are to be implemented
+    // in bug 1074701.
+
+    if (match && match[1]) {
+      conversation.set({loopToken: match[1]});
     }
 
     React.renderComponent(<WebappRootView
       client={client}
       conversation={conversation}
       helper={helper}
       notifications={notifications}
       sdk={OT}
--- a/browser/components/loop/standalone/server.js
+++ b/browser/components/loop/standalone/server.js
@@ -29,36 +29,53 @@ function getConfigFile(req, res) {
     "loop.config.legalWebsiteUrl = '/legal/terms';",
     "loop.config.fxosApp = loop.config.fxosApp || {};",
     "loop.config.fxosApp.name = 'Loop';",
     "loop.config.fxosApp.manifestUrl = 'http://fake-market.herokuapp.com/apps/packagedApp/manifest.webapp';"
   ].join("\n"));
 }
 
 app.get('/content/config.js', getConfigFile);
+app.get('/content/c/config.js', getConfigFile);
 
-// This lets /test/ be mapped to the right place for running tests
-app.use('/', express.static(__dirname + '/../'));
+// Various mappings to let us end up with:
+// /test - for the test files
+// /ui - for the ui showcase
+// /content - for the standalone files.
+
+app.use('/test', express.static(__dirname + '/../test'));
+app.use('/ui', express.static(__dirname + '/../ui'));
+
+// This exists exclusively for the unit tests. They are served the
+// whole loop/ directory structure and expect some files in the standalone directory.
+app.use('/standalone/content', express.static(__dirname + '/content'));
 
-// Magic so that the legal content works both in the standalone server
-// and as static content in the loop-client repo
-app.use('/', express.static(__dirname + '/content/'));
-app.use('/shared', express.static(__dirname + '/../content/shared/'));
-app.get('/config.js', getConfigFile);
+// We load /content this from  both /content *and* /../content. The first one
+// does what we need for running in the github loop-client context, the second one
+// handles running in the hg repo under mozilla-central and is used so that the shared
+// files are in the right location.
+app.use('/content', express.static(__dirname + '/content'));
+app.use('/content', express.static(__dirname + '/../content'));
+// These two are based on the above, but handle call urls, that have a /c/ in them.
+app.use('/content/c', express.static(__dirname + '/content'));
+app.use('/content/c', express.static(__dirname + '/../content'));
 
-// This lets /content/ be mapped right for the static contents.
-app.use('/', express.static(__dirname + '/'));
-// This lets standalone components load images into the UI showcase
-app.use('/standalone/content', express.static(__dirname + '/../content'));
+// As we don't have hashes on the urls, the best way to serve the index files
+// appears to be to be to closely filter the url and match appropriately.
+function serveIndex(req, res) {
+  return res.sendfile(__dirname + '/content/index.html');
+}
+
+app.get(/^\/content\/[\w\-]+$/, serveIndex);
+app.get(/^\/content\/c\/[\w\-]+$/, serveIndex);
 
 var server = app.listen(port);
 
 var baseUrl = "http://localhost:" + port + "/";
 
-console.log("Serving repository root over HTTP at " + baseUrl);
 console.log("Static contents are available at " + baseUrl + "content/");
 console.log("Tests are viewable at " + baseUrl + "test/");
 console.log("Use this for development only.");
 
 // Handle SIGTERM signal.
 function shutdown(cb) {
   "use strict";
 
--- a/browser/components/loop/test/desktop-local/conversation_test.js
+++ b/browser/components/loop/test/desktop-local/conversation_test.js
@@ -74,17 +74,20 @@ describe("loop.conversation", function()
       sandbox.stub(document.mozL10n, "initialize");
 
       sandbox.stub(loop.shared.models.ConversationModel.prototype,
         "initialize");
 
       sandbox.stub(loop.Dispatcher.prototype, "dispatch");
 
       sandbox.stub(loop.shared.utils.Helper.prototype,
-        "locationHash").returns("#incoming/42");
+        "locationData").returns({
+          hash: "#incoming/42",
+          pathname: "/"
+        });
 
       window.OT = {
         overrideGuidStorage: sinon.stub()
       };
     });
 
     afterEach(function() {
       delete window.OT;
@@ -112,18 +115,21 @@ describe("loop.conversation", function()
     describe("when locationHash begins with #room", function () {
       // XXX must stay in sync with "test.alwaysUseRooms" pref check
       // in conversation.jsx:init until we remove that code, which should
       // happen in the second patch in bug 1074686, at which time this comment
       // can go away as well.
       var fakeRoomID = "32";
 
       beforeEach(function() {
-        loop.shared.utils.Helper.prototype.locationHash
-          .returns("#room/" + fakeRoomID);
+        loop.shared.utils.Helper.prototype.locationData
+          .returns({
+            hash: "#room/" + fakeRoomID,
+            pathname: ""
+          });
 
         sandbox.stub(loop.store, "LocalRoomStore");
       });
 
       it("should create a localRoomStore", function() {
         loop.conversation.init();
 
         sinon.assert.calledOnce(loop.store.LocalRoomStore);
@@ -154,17 +160,20 @@ describe("loop.conversation", function()
         new loop.shared.actions.GatherCallData({
           callId: "42",
           outgoing: false
         }));
     });
 
     it("should trigger an outgoing gatherCallData action for outgoing calls",
       function() {
-        loop.shared.utils.Helper.prototype.locationHash.returns("#outgoing/24");
+        loop.shared.utils.Helper.prototype.locationData.returns({
+          hash: "#outgoing/24",
+          pathname: "/"
+        });
 
         loop.conversation.init();
 
         sinon.assert.calledOnce(loop.Dispatcher.prototype.dispatch);
         sinon.assert.calledWithExactly(loop.Dispatcher.prototype.dispatch,
           new loop.shared.actions.GatherCallData({
             callId: "24",
             outgoing: true
--- a/browser/components/loop/test/shared/conversationStore_test.js
+++ b/browser/components/loop/test/shared/conversationStore_test.js
@@ -120,18 +120,21 @@ describe("loop.store.ConversationStore",
         });
       }).to.Throw(/sdkDriver/);
     });
   });
 
   describe("#connectionFailure", function() {
     beforeEach(function() {
       store._websocket = fakeWebsocket;
-      sandbox.stub(loop.shared.utils.Helper.prototype, "locationHash")
-        .returns("#outgoing/42");
+      sandbox.stub(loop.shared.utils.Helper.prototype, "locationData")
+        .returns({
+          hash: "#outgoing/42",
+          pathname: ""
+        });
     });
 
     it("should disconnect the session", function() {
       dispatcher.dispatch(
         new sharedActions.ConnectionFailure({reason: "fake"}));
 
       sinon.assert.calledOnce(sdkDriver.disconnectSession);
     });
@@ -491,18 +494,21 @@ describe("loop.store.ConversationStore",
       wsMediaFailSpy = sinon.spy();
       wsCloseSpy = sinon.spy();
 
       store._websocket = {
         mediaFail: wsMediaFailSpy,
         close: wsCloseSpy
       };
       store.set({callState: CALL_STATES.ONGOING});
-      sandbox.stub(loop.shared.utils.Helper.prototype, "locationHash")
-        .returns("#outgoing/42");
+      sandbox.stub(loop.shared.utils.Helper.prototype, "locationData")
+        .returns({
+          hash: "#outgoing/42",
+          pathname: ""
+        });
     });
 
     it("should disconnect the session", function() {
       dispatcher.dispatch(new sharedActions.HangupCall());
 
       sinon.assert.calledOnce(sdkDriver.disconnectSession);
     });
 
@@ -538,18 +544,21 @@ describe("loop.store.ConversationStore",
       wsMediaFailSpy = sinon.spy();
       wsCloseSpy = sinon.spy();
 
       store._websocket = {
         mediaFail: wsMediaFailSpy,
         close: wsCloseSpy
       };
       store.set({callState: CALL_STATES.ONGOING});
-      sandbox.stub(loop.shared.utils.Helper.prototype, "locationHash")
-        .returns("#outgoing/42");
+      sandbox.stub(loop.shared.utils.Helper.prototype, "locationData")
+        .returns({
+          hash: "#outgoing/42",
+          pathname: ""
+        });
     });
 
     it("should disconnect the session", function() {
       dispatcher.dispatch(new sharedActions.PeerHungupCall());
 
       sinon.assert.calledOnce(sdkDriver.disconnectSession);
     });
 
@@ -573,18 +582,21 @@ describe("loop.store.ConversationStore",
     });
   });
 
   describe("#cancelCall", function() {
     beforeEach(function() {
       store._websocket = fakeWebsocket;
 
       store.set({callState: CALL_STATES.CONNECTING});
-      sandbox.stub(loop.shared.utils.Helper.prototype, "locationHash")
-        .returns("#outgoing/42");
+      sandbox.stub(loop.shared.utils.Helper.prototype, "locationData")
+        .returns({
+          hash: "#outgoing/42",
+          pathname: ""
+        });
     });
 
     it("should disconnect the session", function() {
       dispatcher.dispatch(new sharedActions.CancelCall());
 
       sinon.assert.calledOnce(sdkDriver.disconnectSession);
     });
 
--- a/browser/components/loop/test/standalone/webapp_test.js
+++ b/browser/components/loop/test/standalone/webapp_test.js
@@ -29,40 +29,59 @@ describe("loop.webapp", function() {
     sandbox.restore();
   });
 
   describe("#init", function() {
     var conversationSetStub;
 
     beforeEach(function() {
       sandbox.stub(React, "renderComponent");
-      sandbox.stub(sharedUtils.Helper.prototype,
-                   "locationHash").returns("#call/fake-Token");
       loop.config.feedbackApiUrl = "http://fake.invalid";
       conversationSetStub =
         sandbox.stub(sharedModels.ConversationModel.prototype, "set");
     });
 
     it("should create the WebappRootView", function() {
       loop.webapp.init();
 
       sinon.assert.calledOnce(React.renderComponent);
       sinon.assert.calledWith(React.renderComponent,
         sinon.match(function(value) {
           return TestUtils.isDescriptorOfType(value,
             loop.webapp.WebappRootView);
       }));
     });
 
-    it("should set the loopToken on the conversation", function() {
-      loop.webapp.init();
+    it("should set the loopToken on the conversation for old-style call urls",
+      function() {
+        sandbox.stub(sharedUtils.Helper.prototype,
+          "locationData").returns({
+            hash: "#call/fake-Token",
+            pathname: "/"
+          });
+
+        loop.webapp.init();
+
+        sinon.assert.called(conversationSetStub);
+        sinon.assert.calledWithExactly(conversationSetStub, {loopToken: "fake-Token"});
+      });
 
-       sinon.assert.called(conversationSetStub);
-       sinon.assert.calledWithExactly(conversationSetStub, "loopToken", "fake-Token");
-    });
+    it("should set the loopToken on the conversation for new-style call urls",
+      function() {
+        sandbox.stub(sharedUtils.Helper.prototype,
+          "locationData").returns({
+            hash: "",
+            pathname: "/c/abc123-_Tes"
+          });
+
+        loop.webapp.init();
+
+        sinon.assert.called(conversationSetStub);
+        sinon.assert.calledWithExactly(conversationSetStub, {loopToken: "abc123-_Tes"});
+      });
   });
 
   describe("OutgoingConversationView", function() {
     var ocView, conversation, client;
 
     function mountTestComponent(props) {
       return TestUtils.renderIntoDocument(
         loop.webapp.OutgoingConversationView(props));
--- a/browser/components/loop/ui/index.html
+++ b/browser/components/loop/ui/index.html
@@ -6,17 +6,17 @@
   <head>
     <meta charset="utf-8">
     <title>Loop UI Components Showcase</title>
     <link rel="stylesheet" type="text/css" href="../content/shared/css/reset.css">
     <link rel="stylesheet" type="text/css" href="../content/shared/css/common.css">
     <link rel="stylesheet" type="text/css" href="../content/shared/css/conversation.css">
     <link rel="stylesheet" type="text/css" href="../content/shared/css/panel.css">
     <link rel="stylesheet" type="text/css" href="../content/shared/css/contacts.css">
-    <link rel="stylesheet" type="text/css" href="../standalone/content/css/webapp.css">
+    <link rel="stylesheet" type="text/css" href="../content/css/webapp.css">
     <link rel="stylesheet" type="text/css" href="ui-showcase.css">
  </head>
   <body>
     <div id="main"></div>
     <script src="fake-mozLoop.js"></script>
     <script src="fake-l10n.js"></script>
     <script>
       window.OTProperties = {
@@ -40,17 +40,17 @@
     <script src="../content/shared/js/websocket.js"></script>
     <script src="../content/shared/js/validate.js"></script>
     <script src="../content/shared/js/dispatcher.js"></script>
     <script src="../content/shared/js/conversationStore.js"></script>
     <script src="../content/shared/js/roomListStore.js"></script>
     <script src="../content/js/roomViews.js"></script>
     <script src="../content/js/conversationViews.js"></script>
     <script src="../content/js/client.js"></script>
-    <script src="../standalone/content/js/webapp.js"></script>
+    <script src="../content/js/webapp.js"></script>
     <script type="text/javascript;version=1.8" src="../content/js/contacts.js"></script>
     <script>
       if (!loop.contacts) {
         // For browsers that don't support ES6 without special flags (all but Fx
         // at the moment), we shim the contacts namespace with its most barebone
         // implementation.
         loop.contacts = {
           ContactsList: React.createClass({render: function() {