From c254d043c5f69838be4feab0fc08fe652a691ddc Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Apr 2018 03:58:17 +0100 Subject: [PATCH 1/7] fix ugly img errors and correctly render SVG thumbnails Fixes https://github.com/vector-im/riot-web/issues/6271 Fixes https://github.com/vector-im/riot-web/issues/1341 --- src/components/views/messages/MImageBody.js | 60 ++++++++++----------- 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index d5ce533bda7..7ecbf913651 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -154,7 +154,16 @@ export default class extends React.Component { return this.state.decryptedThumbnailUrl; } return this.state.decryptedUrl; - } else { + } + 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); } } @@ -230,6 +239,9 @@ 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. + //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,18 +252,22 @@ export default class extends React.Component { } _messageContent(contentUrl, thumbUrl, content) { + const thumbnail = ( + + {content.body} + + ); + return ( - - {content.body} - + { thumbUrl && !this.state.imgError ? thumbnail : '' } - + ); } @@ -286,14 +302,6 @@ export default class extends React.Component { ); } - if (this.state.imgError) { - return ( - - { _t("This image cannot be displayed.") } - - ); - } - const contentUrl = this._getContentUrl(); let thumbUrl; if (this._isGif() && SettingsStore.getValue("autoplayGifsAndVideos")) { @@ -302,20 +310,6 @@ export default class extends React.Component { thumbUrl = this._getThumbUrl(); } - if (thumbUrl) { - return this._messageContent(contentUrl, thumbUrl, content); - } else if (content.body) { - return ( - - { _t("Image '%(Body)s' cannot be displayed.", {Body: content.body}) } - - ); - } else { - return ( - - { _t("This image cannot be displayed.") } - - ); - } + return this._messageContent(contentUrl, thumbUrl, content); } } From 731f1fa7d3a91b90555e9211db692c7bf1c57820 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Apr 2018 04:00:02 +0100 Subject: [PATCH 2/7] clarify another scrolljump bug --- src/components/views/messages/MImageBody.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index 7ecbf913651..034e700793c 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -242,6 +242,8 @@ export default class extends React.Component { // 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 + //console.log("trying to fit image into timelineWidth of " + this.refs.body.offsetWidth + " or " + this.refs.body.clientWidth); let thumbHeight = null; if (content.info) { From 551d3ebda0e78c4a0d505d1cb12705a8edbad71c Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Apr 2018 04:28:15 +0100 Subject: [PATCH 3/7] correctly fix up thumbnail height onload. fixes https://github.com/vector-im/riot-web/issues/6492, although popping is inevitable in the current implementation as it only fixes up the thumbnail size once the image has loaded. --- src/components/views/messages/MImageBody.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index 034e700793c..99135fdf8c8 100644 --- a/src/components/views/messages/MImageBody.js +++ b/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) { @@ -170,7 +176,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); @@ -192,7 +197,6 @@ export default class extends React.Component { decryptedThumbnailUrl: thumbnailUrl, decryptedBlob: decryptedBlob, }); - this.props.onWidgetLoad(); }); }).catch((err) => { console.warn("Unable to decrypt attachment: ", err); @@ -244,7 +248,7 @@ export default class extends React.Component { // FIXME: It will also break really badly for images with broken or missing thumbnails - //console.log("trying to fit image into timelineWidth of " + this.refs.body.offsetWidth + " or " + this.refs.body.clientWidth); + // console.log("trying to fit image into timelineWidth of " + this.refs.body.offsetWidth + " or " + this.refs.body.clientWidth); let thumbHeight = null; if (content.info) { thumbHeight = ImageUtils.thumbHeight(content.info.w, content.info.h, timelineWidth, maxHeight); @@ -259,7 +263,7 @@ export default class extends React.Component { {content.body} From be523b3edc4259fd19a3fcd306af80fb463744ed Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Apr 2018 04:31:30 +0100 Subject: [PATCH 4/7] lint --- src/components/views/messages/MImageBody.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index 99135fdf8c8..b02c4586424 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -160,16 +160,14 @@ 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) { + } 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 + content.info.thumbnail_url, 800, 600, ); - } - else { + } else { return this.context.matrixClient.mxcUrlToHttp(content.url, 800, 600); } } From 8538cc1666eaba2d8792ba18b870b554040e6eea Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Apr 2018 04:41:30 +0100 Subject: [PATCH 5/7] fix regressions introduced by https://github.com/vector-im/riot-web/commit/00b7cc512b3a9f0e0a36e4ea7d1f4a6c3011ff66 --- res/css/structures/_FilePanel.scss | 4 ++-- src/components/views/messages/MFileBody.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/res/css/structures/_FilePanel.scss b/res/css/structures/_FilePanel.scss index 58e090645fe..87dc0aa7561 100644 --- a/res/css/structures/_FilePanel.scss +++ b/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; } diff --git a/src/components/views/messages/MFileBody.js b/src/components/views/messages/MFileBody.js index d9ce61582e1..246ea6891fa 100644 --- a/src/components/views/messages/MFileBody.js +++ b/src/components/views/messages/MFileBody.js @@ -361,7 +361,7 @@ module.exports = React.createClass({ return (
- + { fileName }
From 45c0c0eddd0744bb337a18a0d50179802b26658a Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Apr 2018 04:50:44 +0100 Subject: [PATCH 6/7] add CSS destroyed by stickerpacks fixing mangling from https://github.com/vector-im/riot-web/commit/9fc7435ea2ef7af161fadf4b3f9abc4a4aadcb73#diff-be2f1d72c9704840ceddf019c825e825 and https://github.com/vector-im/riot-web/commit/38efebb8d3946f321e6814cbd957afd042505538#diff-be2f1d72c9704840ceddf019c825e825 also fixes https://github.com/vector-im/riot-web/issues/6492 --- res/css/views/messages/_MImageBody.scss | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/res/css/views/messages/_MImageBody.scss b/res/css/views/messages/_MImageBody.scss index bf483feda33..1c809f0743d 100644 --- a/res/css/views/messages/_MImageBody.scss +++ b/res/css/views/messages/_MImageBody.scss @@ -15,6 +15,10 @@ limitations under the License. */ .mx_MImageBody { - display: block; - margin-right: 34px; + display: block; + margin-right: 34px; } + +.mx_MImageBody_thumbnail { + max-width: 100%; +} \ No newline at end of file From db5fc53853951238dd0b44df833ae9578819f8d4 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 29 Apr 2018 04:53:32 +0100 Subject: [PATCH 7/7] final comment --- src/components/views/messages/MImageBody.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index b02c4586424..2372c38f4bc 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -246,6 +246,10 @@ export default class extends React.Component { // 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) {