Bug 1146305 - IDN links should be properly displayed in Hello rooms for context in conversations. r=mikedeboer
authorMark Banner <standard8@mozilla.com>
Thu, 30 Apr 2015 07:40:49 +0100
changeset 241821 511678a783bc29cfb1f718e5e8baf61513f7f271
parent 241820 6eb3e36c9795cfb5a226b329adc46a3a7e5972b5
child 241822 e608444e72aabd5c585d99d337307e00b196994a
push id28671
push userryanvm@gmail.com
push dateFri, 01 May 2015 14:27:58 +0000
treeherdermozilla-central@60b269fed8cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1146305
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1146305 - IDN links should be properly displayed in Hello rooms for context in conversations. r=mikedeboer
browser/components/loop/content/js/roomViews.js
browser/components/loop/content/js/roomViews.jsx
browser/components/loop/content/shared/js/utils.js
browser/components/loop/standalone/content/js/standaloneRoomViews.js
browser/components/loop/standalone/content/js/standaloneRoomViews.jsx
browser/components/loop/test/desktop-local/roomViews_test.js
browser/components/loop/test/shared/utils_test.js
browser/components/loop/test/standalone/standaloneRoomViews_test.js
--- a/browser/components/loop/content/js/roomViews.js
+++ b/browser/components/loop/content/js/roomViews.js
@@ -6,20 +6,21 @@
 
 /* jshint newcap:false */
 /* global loop:true, React */
 
 var loop = loop || {};
 loop.roomViews = (function(mozL10n) {
   "use strict";
 
+  var ROOM_STATES = loop.store.ROOM_STATES;
+  var SCREEN_SHARE_STATES = loop.shared.utils.SCREEN_SHARE_STATES;
   var sharedActions = loop.shared.actions;
   var sharedMixins = loop.shared.mixins;
-  var ROOM_STATES = loop.store.ROOM_STATES;
-  var SCREEN_SHARE_STATES = loop.shared.utils.SCREEN_SHARE_STATES;
+  var sharedUtils = loop.shared.utils;
   var sharedViews = loop.shared.views;
 
   /**
    * ActiveRoomStore mixin.
    * @type {Object}
    */
   var ActiveRoomStoreMixin = {
     mixins: [Backbone.Events],
@@ -307,23 +308,35 @@ loop.roomViews = (function(mozL10n) {
     render: function() {
       if (!this.state.show)
         return null;
 
       var URL = this.props.roomData.roomContextUrls && this.props.roomData.roomContextUrls[0];
       var thumbnail = URL && URL.thumbnail || "";
       var URLDescription = URL && URL.description || "";
       var location = URL && URL.location || "";
+      var locationData = null;
+      if (location) {
+        locationData = sharedUtils.formatURL(location);
+      }
+
+      if (!locationData) {
+        return null;
+      }
+
       return (
         React.createElement("div", {className: "room-context"}, 
           React.createElement("img", {className: "room-context-thumbnail", src: thumbnail}), 
           React.createElement("div", {className: "room-context-content"}, 
             React.createElement("div", {className: "room-context-label"}, mozL10n.get("context_inroom_label")), 
             React.createElement("div", {className: "room-context-description"}, URLDescription), 
-            React.createElement("a", {className: "room-context-url", href: location, target: "_blank"}, location), 
+            React.createElement("a", {className: "room-context-url", 
+               href: location, 
+               target: "_blank", 
+               title: locationData.location}, locationData.hostname), 
             this.props.roomData.roomDescription ?
               React.createElement("div", {className: "room-context-comment"}, this.props.roomData.roomDescription) :
               null, 
             React.createElement("button", {className: "room-context-btn-close", 
                     onClick: this.handleCloseClick})
           )
         )
       );
--- a/browser/components/loop/content/js/roomViews.jsx
+++ b/browser/components/loop/content/js/roomViews.jsx
@@ -6,20 +6,21 @@
 
 /* jshint newcap:false */
 /* global loop:true, React */
 
 var loop = loop || {};
 loop.roomViews = (function(mozL10n) {
   "use strict";
 
+  var ROOM_STATES = loop.store.ROOM_STATES;
+  var SCREEN_SHARE_STATES = loop.shared.utils.SCREEN_SHARE_STATES;
   var sharedActions = loop.shared.actions;
   var sharedMixins = loop.shared.mixins;
-  var ROOM_STATES = loop.store.ROOM_STATES;
-  var SCREEN_SHARE_STATES = loop.shared.utils.SCREEN_SHARE_STATES;
+  var sharedUtils = loop.shared.utils;
   var sharedViews = loop.shared.views;
 
   /**
    * ActiveRoomStore mixin.
    * @type {Object}
    */
   var ActiveRoomStoreMixin = {
     mixins: [Backbone.Events],
@@ -307,23 +308,35 @@ loop.roomViews = (function(mozL10n) {
     render: function() {
       if (!this.state.show)
         return null;
 
       var URL = this.props.roomData.roomContextUrls && this.props.roomData.roomContextUrls[0];
       var thumbnail = URL && URL.thumbnail || "";
       var URLDescription = URL && URL.description || "";
       var location = URL && URL.location || "";
+      var locationData = null;
+      if (location) {
+        locationData = sharedUtils.formatURL(location);
+      }
+
+      if (!locationData) {
+        return null;
+      }
+
       return (
         <div className="room-context">
           <img className="room-context-thumbnail" src={thumbnail}/>
           <div className="room-context-content">
             <div className="room-context-label">{mozL10n.get("context_inroom_label")}</div>
             <div className="room-context-description">{URLDescription}</div>
-            <a className="room-context-url" href={location} target="_blank">{location}</a>
+            <a className="room-context-url"
+               href={location}
+               target="_blank"
+               title={locationData.location}>{locationData.hostname}</a>
             {this.props.roomData.roomDescription ?
               <div className="room-context-comment">{this.props.roomData.roomDescription}</div> :
               null}
             <button className="room-context-btn-close"
                     onClick={this.handleCloseClick}/>
           </div>
         </div>
       );
--- a/browser/components/loop/content/shared/js/utils.js
+++ b/browser/components/loop/content/shared/js/utils.js
@@ -267,16 +267,43 @@ var inChrome = typeof Components != "und
   function locationData() {
     return {
       hash: window.location.hash,
       pathname: window.location.pathname
     };
   }
 
   /**
+   * Formats a url for display purposes. This includes converting the
+   * domain to punycode, and then decoding the url.
+   *
+   * @param {String} url The url to format.
+   * @return {Object}    An object containing the hostname and full location.
+   */
+  function formatURL(url) {
+    // We're using new URL to pass this through the browser's ACE/punycode
+    // processing system. If the browser considers a url to need to be
+    // punycode encoded for it to be displayed, then new URL will do that for
+    // us. This saves us needing our own punycode library.
+    var urlObject;
+    try {
+      urlObject = new URL(url);
+    } catch (ex) {
+      console.error("Error occurred whilst parsing URL:", ex);
+      return null;
+    }
+
+    // Finally, ensure we look good.
+    return {
+      hostname: urlObject.hostname,
+      location: decodeURI(urlObject.href)
+    };
+  }
+
+  /**
    * Generates and opens a mailto: url with call URL information prefilled.
    * Note: This only works for Desktop.
    *
    * @param  {String} callUrl   The call URL.
    * @param  {String} recipient The recipient email address (optional).
    */
   function composeCallUrlEmail(callUrl, recipient) {
     if (typeof navigator.mozLoop === "undefined") {
@@ -531,16 +558,17 @@ var inChrome = typeof Components != "und
     FAILURE_DETAILS: FAILURE_DETAILS,
     REST_ERRNOS: REST_ERRNOS,
     WEBSOCKET_REASONS: WEBSOCKET_REASONS,
     STREAM_PROPERTIES: STREAM_PROPERTIES,
     SCREEN_SHARE_STATES: SCREEN_SHARE_STATES,
     ROOM_INFO_FAILURES: ROOM_INFO_FAILURES,
     composeCallUrlEmail: composeCallUrlEmail,
     formatDate: formatDate,
+    formatURL: formatURL,
     getBoolPreference: getBoolPreference,
     getOS: getOS,
     getOSVersion: getOSVersion,
     isChrome: isChrome,
     isFirefox: isFirefox,
     isFirefoxOS: isFirefoxOS,
     isOpera: isOpera,
     getUnsupportedPlatform: getUnsupportedPlatform,
--- a/browser/components/loop/standalone/content/js/standaloneRoomViews.js
+++ b/browser/components/loop/standalone/content/js/standaloneRoomViews.js
@@ -11,16 +11,17 @@ var loop = loop || {};
 loop.standaloneRoomViews = (function(mozL10n) {
   "use strict";
 
   var FAILURE_DETAILS = loop.shared.utils.FAILURE_DETAILS;
   var ROOM_INFO_FAILURES = loop.shared.utils.ROOM_INFO_FAILURES;
   var ROOM_STATES = loop.store.ROOM_STATES;
   var sharedActions = loop.shared.actions;
   var sharedMixins = loop.shared.mixins;
+  var sharedUtils = loop.shared.utils;
   var sharedViews = loop.shared.views;
 
   var StandaloneRoomInfoArea = React.createClass({displayName: "StandaloneRoomInfoArea",
     propTypes: {
       isFirefox: React.PropTypes.bool.isRequired,
       activeRoomStore: React.PropTypes.oneOfType([
         React.PropTypes.instanceOf(loop.store.ActiveRoomStore),
         React.PropTypes.instanceOf(loop.store.FxOSActiveRoomStore)
@@ -243,33 +244,37 @@ loop.standaloneRoomViews = (function(moz
     },
 
     render: function() {
       if (!this.props.roomContextUrl ||
           !this.props.roomContextUrl.location) {
         return null;
       }
 
-      var location = this.props.roomContextUrl.location;
+      var locationInfo = sharedUtils.formatURL(this.props.roomContextUrl.location);
+      if (!locationInfo) {
+        return null;
+      }
 
       var cx = React.addons.classSet;
 
       var classes = cx({
         "standalone-context-url": true,
         "screen-share-active": this.props.receivingScreenShare
       });
 
       return (
         React.createElement("div", {className: classes}, 
             React.createElement("img", {src: this.props.roomContextUrl.thumbnail}), 
           React.createElement("div", {className: "standalone-context-url-description-wrapper"}, 
             this.props.roomContextUrl.description, 
-            React.createElement("br", null), React.createElement("a", {href: location, 
+            React.createElement("br", null), React.createElement("a", {href: locationInfo.location, 
                      onClick: this.recordClick, 
-                     target: "_blank"}, location)
+                     target: "_blank", 
+                     title: locationInfo.location}, locationInfo.hostname)
           )
         )
       );
     }
   });
 
   var StandaloneRoomContextView = React.createClass({displayName: "StandaloneRoomContextView",
     propTypes: {
--- a/browser/components/loop/standalone/content/js/standaloneRoomViews.jsx
+++ b/browser/components/loop/standalone/content/js/standaloneRoomViews.jsx
@@ -11,16 +11,17 @@ var loop = loop || {};
 loop.standaloneRoomViews = (function(mozL10n) {
   "use strict";
 
   var FAILURE_DETAILS = loop.shared.utils.FAILURE_DETAILS;
   var ROOM_INFO_FAILURES = loop.shared.utils.ROOM_INFO_FAILURES;
   var ROOM_STATES = loop.store.ROOM_STATES;
   var sharedActions = loop.shared.actions;
   var sharedMixins = loop.shared.mixins;
+  var sharedUtils = loop.shared.utils;
   var sharedViews = loop.shared.views;
 
   var StandaloneRoomInfoArea = React.createClass({
     propTypes: {
       isFirefox: React.PropTypes.bool.isRequired,
       activeRoomStore: React.PropTypes.oneOfType([
         React.PropTypes.instanceOf(loop.store.ActiveRoomStore),
         React.PropTypes.instanceOf(loop.store.FxOSActiveRoomStore)
@@ -243,33 +244,37 @@ loop.standaloneRoomViews = (function(moz
     },
 
     render: function() {
       if (!this.props.roomContextUrl ||
           !this.props.roomContextUrl.location) {
         return null;
       }
 
-      var location = this.props.roomContextUrl.location;
+      var locationInfo = sharedUtils.formatURL(this.props.roomContextUrl.location);
+      if (!locationInfo) {
+        return null;
+      }
 
       var cx = React.addons.classSet;
 
       var classes = cx({
         "standalone-context-url": true,
         "screen-share-active": this.props.receivingScreenShare
       });
 
       return (
         <div className={classes}>
             <img src={this.props.roomContextUrl.thumbnail} />
           <div className="standalone-context-url-description-wrapper">
             {this.props.roomContextUrl.description}
-            <br /><a href={location}
+            <br /><a href={locationInfo.location}
                      onClick={this.recordClick}
-                     target="_blank">{location}</a>
+                     target="_blank"
+                     title={locationInfo.location}>{locationInfo.hostname}</a>
           </div>
         </div>
       );
     }
   });
 
   var StandaloneRoomContextView = React.createClass({
     propTypes: {
--- a/browser/components/loop/test/desktop-local/roomViews_test.js
+++ b/browser/components/loop/test/desktop-local/roomViews_test.js
@@ -237,16 +237,33 @@ describe("loop.roomViews", function () {
           showContext: true,
           roomData: {
             roomContextUrls: [fakeContextURL]
           }
         });
 
         expect(view.getDOMNode().querySelector(".room-context")).to.not.eql(null);
       });
+
+      it("should format the context url for display", function() {
+        sandbox.stub(sharedUtils, "formatURL").returns({
+          location: "location",
+          hostname: "hostname"
+        });
+
+        view = mountTestComponent({
+          showContext: true,
+          roomData: {
+            roomContextUrls: [fakeContextURL]
+          }
+        });
+
+        expect(view.getDOMNode().querySelector(".room-context-url").textContent)
+          .eql("hostname");
+      });
     });
   });
 
   describe("DesktopRoomConversationView", function() {
     var view;
 
     beforeEach(function() {
       loop.store.StoreMixin.register({
--- a/browser/components/loop/test/shared/utils_test.js
+++ b/browser/components/loop/test/shared/utils_test.js
@@ -140,16 +140,41 @@ describe("loop.shared.utils", function()
       it("should return the localStorage value", function() {
         localStorage.setItem("test.true", true);
 
         expect(sharedUtils.getBoolPreference("test.true")).eql(true);
       });
     });
   });
 
+  describe("#formatURL", function() {
+    it("should decode encoded URIs", function() {
+      expect(sharedUtils.formatURL("http://invalid.com/?a=Foo%20Bar"))
+        .eql({
+          location: "http://invalid.com/?a=Foo Bar",
+          hostname: "invalid.com"
+        });
+    });
+
+    it("should change some idn urls to ascii encoded", function() {
+      // Note, this is based on the browser's list of what does/doesn't get
+      // altered for punycode, so if the list changes this could change in the
+      // future.
+      expect(sharedUtils.formatURL("http://\u0261oogle.com/"))
+        .eql({
+          location: "http://xn--oogle-qmc.com/",
+          hostname: "xn--oogle-qmc.com"
+        });
+    });
+
+    it("should return null if it the url is not valid", function() {
+      expect(sharedUtils.formatURL("hinvalid//url")).eql(null);
+    });
+  });
+
   describe("#composeCallUrlEmail", function() {
     var composeEmail;
 
     beforeEach(function() {
       // fake mozL10n
       sandbox.stub(navigator.mozL10n, "get", function(id) {
         switch(id) {
           case "share_email_subject5": return "subject";
--- a/browser/components/loop/test/standalone/standaloneRoomViews_test.js
+++ b/browser/components/loop/test/standalone/standaloneRoomViews_test.js
@@ -8,16 +8,17 @@ var expect = chai.expect;
 
 describe("loop.standaloneRoomViews", function() {
   "use strict";
 
   var ROOM_STATES = loop.store.ROOM_STATES;
   var FEEDBACK_STATES = loop.store.FEEDBACK_STATES;
   var ROOM_INFO_FAILURES = loop.shared.utils.ROOM_INFO_FAILURES;
   var sharedActions = loop.shared.actions;
+  var sharedUtils = loop.shared.utils;
 
   var sandbox, dispatcher, activeRoomStore, feedbackStore, dispatch;
 
   beforeEach(function() {
     sandbox = sinon.sandbox.create();
     dispatcher = new loop.Dispatcher();
     dispatch = sandbox.stub(dispatcher, "dispatch");
     activeRoomStore = new loop.store.ActiveRoomStore(dispatcher, {
@@ -91,16 +92,36 @@ describe("loop.standaloneRoomViews", fun
           location: "http://invalid.com",
           thumbnail: "data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=="
         }]
       });
 
       expect(view.getDOMNode().querySelector(".standalone-context-url")).not.eql(null);
     });
 
+    it("should format the url for display", function() {
+      sandbox.stub(sharedUtils, "formatURL").returns({
+          location: "location",
+          hostname: "hostname"
+        });
+
+      var view = mountTestComponent({
+        roomName: "Mike's room",
+        roomContextUrls: [{
+          description: "Mark's super page",
+          location: "http://invalid.com",
+          thumbnail: "data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=="
+        }]
+      });
+
+      expect(view.getDOMNode()
+        .querySelector(".standalone-context-url-description-wrapper > a").textContent)
+        .eql("hostname");
+    });
+
     it("should not display context information if no urls are supplied", function() {
       var view = mountTestComponent({
         roomName: "Mike's room"
       });
 
       expect(view.getDOMNode().querySelector(".standalone-context-url")).eql(null);
     });