Merge pull request #1865 from matrix-org/matthew/image_errors

fix ugly img errors and correctly render SVG thumbnails

res/css/structures/_FilePanel.scss

@@ -50,13 +50,13 @@ limitations under the License.
     margin-right: 0px;
 }
 
-.mx_FilePanel .mx_EventTile .mx_MImageBody_download {
+.mx_FilePanel .mx_EventTile .mx_MFileBody_download {
     display: flex;
     font-size: 14px;
     color: $event-timestamp-color;
 }
 
-.mx_FilePanel .mx_EventTile .mx_MImageBody_downloadLink {
+.mx_FilePanel .mx_EventTile .mx_MFileBody_downloadLink {
     flex: 1 1 auto;
     color: $light-fg-color;
 }

res/css/views/messages/_MImageBody.scss

@@ -18,3 +18,7 @@ limitations under the License.
     display: block;
     margin-right: 34px;
 }
+
+.mx_MImageBody_thumbnail {
+    max-width: 100%;
+}

src/components/views/messages/MFileBody.js

@@ -361,7 +361,7 @@ module.exports = React.createClass({
                 return (
                     <span className="mx_MFileBody">
                         <div className="mx_MFileBody_download">
-                            <a className="mx_ImageBody_downloadLink" href={contentUrl} download={fileName} target="_blank">
+                            <a className="mx_MFileBody_downloadLink" href={contentUrl} download={fileName} target="_blank">
                                 { fileName }
                             </a>
                             <div className="mx_MImageBody_size">

src/components/views/messages/MImageBody.js

@@ -50,6 +50,7 @@ export default class extends React.Component {
 
         this.onAction = this.onAction.bind(this);
         this.onImageError = this.onImageError.bind(this);
+        this.onImageLoad = this.onImageLoad.bind(this);
         this.onImageEnter = this.onImageEnter.bind(this);
         this.onImageLeave = this.onImageLeave.bind(this);
         this.onClientSync = this.onClientSync.bind(this);
@@ -137,6 +138,11 @@ export default class extends React.Component {
         });
     }
 
+    onImageLoad() {
+        this.fixupHeight();
+        this.props.onWidgetLoad();
+    }
+
     _getContentUrl() {
         const content = this.props.mxEvent.getContent();
         if (content.file !== undefined) {
@@ -154,6 +160,13 @@ export default class extends React.Component {
                 return this.state.decryptedThumbnailUrl;
             }
             return this.state.decryptedUrl;
+        } else if (content.info.mimetype == "image/svg+xml" && content.info.thumbnail_url) {
+            // special case to return client-generated thumbnails for SVGs, if any,
+            // given we deliberately don't thumbnail them serverside to prevent
+            // billion lol attacks and similar
+            return this.context.matrixClient.mxcUrlToHttp(
+                content.info.thumbnail_url, 800, 600,
+            );
         } else {
             return this.context.matrixClient.mxcUrlToHttp(content.url, 800, 600);
         }
@@ -161,7 +174,6 @@ export default class extends React.Component {
 
     componentDidMount() {
         this.dispatcherRef = dis.register(this.onAction);
-        this.fixupHeight();
         const content = this.props.mxEvent.getContent();
         if (content.file !== undefined && this.state.decryptedUrl === null) {
             let thumbnailPromise = Promise.resolve(null);
@@ -183,7 +195,6 @@ export default class extends React.Component {
                         decryptedThumbnailUrl: thumbnailUrl,
                         decryptedBlob: decryptedBlob,
                     });
-                    this.props.onWidgetLoad();
                 });
             }).catch((err) => {
                 console.warn("Unable to decrypt attachment: ", err);
@@ -230,6 +241,15 @@ export default class extends React.Component {
         const maxHeight = 600; // let images take up as much width as they can so long as the height doesn't exceed 600px.
         // the alternative here would be 600*timelineWidth/800; to scale them down to fit inside a 4:3 bounding box
 
+        // FIXME: this will break on clientside generated thumbnails (as per e2e rooms)
+        // which may well be much smaller than the 800x600 bounding box.
+
+        // FIXME: It will also break really badly for images with broken or missing thumbnails
+
+        // FIXME: Because we don't know what size of thumbnail the server's actually going to send
+        // us, we can't even really layout the page nicely for it.  Instead we have to assume
+        // it'll target 800x600 and we'll downsize if needed to make things fit.
+
         // console.log("trying to fit image into timelineWidth of " + this.refs.body.offsetWidth + " or " + this.refs.body.clientWidth);
         let thumbHeight = null;
         if (content.info) {
@@ -240,16 +260,20 @@ export default class extends React.Component {
     }
 
     _messageContent(contentUrl, thumbUrl, content) {
-        return (
-            <span className="mx_MImageBody" ref="body">
+        const thumbnail = (
             <a href={contentUrl} onClick={this.onClick}>
                 <img className="mx_MImageBody_thumbnail" src={thumbUrl} ref="image"
                     alt={content.body}
                     onError={this.onImageError}
-                        onLoad={this.props.onWidgetLoad}
+                    onLoad={this.onImageLoad}
                     onMouseEnter={this.onImageEnter}
                     onMouseLeave={this.onImageLeave} />
             </a>
+        );
+
+        return (
+            <span className="mx_MImageBody" ref="body">
+                { thumbUrl && !this.state.imgError ? thumbnail : '' }
                 <MFileBody {...this.props} decryptedBlob={this.state.decryptedBlob} />
             </span>
       );
@@ -286,14 +310,6 @@ export default class extends React.Component {
             );
         }
 
-        if (this.state.imgError) {
-            return (
-                <span className="mx_MImageBody">
-                    { _t("This image cannot be displayed.") }
-                </span>
-            );
-        }
-
         const contentUrl = this._getContentUrl();
         let thumbUrl;
         if (this._isGif() && SettingsStore.getValue("autoplayGifsAndVideos")) {
@@ -302,20 +318,6 @@ export default class extends React.Component {
           thumbUrl = this._getThumbUrl();
         }
 
-        if (thumbUrl) {
         return this._messageContent(contentUrl, thumbUrl, content);
-        } else if (content.body) {
-            return (
-                <span className="mx_MImageBody">
-                    { _t("Image '%(Body)s' cannot be displayed.", {Body: content.body}) }
-                </span>
-            );
-        } else {
-            return (
-                <span className="mx_MImageBody">
-                    { _t("This image cannot be displayed.") }
-                </span>
-            );
-        }
     }
 }