From c8d233c0a6178020d6fc77c4dbcb9a6240b45eac Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 14 Jun 2018 14:19:30 +0100 Subject: [PATCH 1/3] If unspecified, don't crash if missing thumbnail info applies to stickers/images. We might want to consider to do that is better than assuming a aspect ratio of 600 x 800 (4:3). --- src/components/views/messages/MImageBody.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index c42840b03a..8ab3705766 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -185,7 +185,7 @@ export default class extends React.Component { const content = this.props.mxEvent.getContent(); if (content.file !== undefined && this.state.decryptedUrl === null) { let thumbnailPromise = Promise.resolve(null); - if (content.info.thumbnail_file) { + if (content.info && content.info.thumbnail_file) { thumbnailPromise = decryptFile( content.info.thumbnail_file, ).then(function(blob) { @@ -239,11 +239,20 @@ export default class extends React.Component { } _messageContent(contentUrl, thumbUrl, content) { + // If unspecifide in the thumbnail info, assume width x height to be 800 x 600. + let infoHeight = 600; + let infoWidth = 800; + + if (content.info && content.info.h && content.info.w) { + infoHeight = content.info.h; + infoWidth = content.info.w; + } + // The maximum height of the thumbnail as it is rendered as an - const maxHeight = Math.min(this.props.maxImageHeight || 600, content.info.h); + const maxHeight = Math.min(this.props.maxImageHeight || 600, infoHeight); // The maximum width of the thumbnail, as dictated by its natural // maximum height. - const maxWidth = content.info.w * maxHeight / content.info.h; + const maxWidth = infoWidth * maxHeight / infoHeight; let img = null; let placeholder = null; @@ -274,12 +283,12 @@ export default class extends React.Component { const thumbnail = (
{ /* Calculate aspect ratio, using %padding will size _container correctly */ } -
+
{ showPlaceholder &&
{ placeholder } From 2d14d51ecb6bbe9f5922f4144054457f456151c5 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 14 Jun 2018 15:44:55 +0100 Subject: [PATCH 2/3] Handle images without width/height info correctly Prior to #1912, height fix up of image events without an `info` in their content would fail, setting `style.height = null + "px"`. Now that all thumbnail sizing is done through one path, we can fix the same problem for all cases (images, stickers, e2e/non-e2e) by handling images without `info` correctly. At the bare minimum, we use a null-guard that will make sure an image without an `info` does not appear in the timeline (as a spinner or otherwise until loaded). When loaded, we size it like any other image by using the natural dimensions of the loaded image in place of `info`. Note that we do not apply the same logic to images that *do* specify an `info` with `w` and `h` keys. If the aspect ratio of the image does not match that of the event, we use the one in `info` even when the image has loaded. --- src/components/views/messages/MImageBody.js | 38 +++++++++++++++++---- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index 8ab3705766..c13f963545 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -147,7 +147,16 @@ export default class extends React.Component { onImageLoad() { this.props.onWidgetLoad(); - this.setState({ imgLoaded: true }); + + let loadedImageDimensions; + + if (this.refs.image) { + const { naturalWidth, naturalHeight } = this.refs.image; + + loadedImageDimensions = { naturalWidth, naturalHeight }; + } + + this.setState({ imgLoaded: true, loadedImageDimensions }); } _getContentUrl() { @@ -239,13 +248,30 @@ export default class extends React.Component { } _messageContent(contentUrl, thumbUrl, content) { - // If unspecifide in the thumbnail info, assume width x height to be 800 x 600. - let infoHeight = 600; - let infoWidth = 800; + let infoWidth; + let infoHeight; - if (content.info && content.info.h && content.info.w) { - infoHeight = content.info.h; + if (content && content.info && content.info.w && content.info.h) { infoWidth = content.info.w; + infoHeight = content.info.h; + } else { + // Whilst the image loads, display nothing. + // + // Once loaded, use the loaded image dimensions stored in `loadedImageDimensions`. + // + // By doing this, the image "pops" into the timeline, but is still restricted + // by the same width and height logic below. + if (!this.state.loadedImageDimensions) { + return this.wrapImage(contentUrl, + {content.body}, + ); + } + infoWidth = this.state.loadedImageDimensions.naturalWidth; + infoHeight = this.state.loadedImageDimensions.naturalHeight; } // The maximum height of the thumbnail as it is rendered as an From 2eb23ed234c6114658dc5fe6829884db3a6beea1 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 14 Jun 2018 15:53:49 +0100 Subject: [PATCH 3/3] Add loadedImageDimensions to initial state --- src/components/views/messages/MImageBody.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index c13f963545..1b6bdeb588 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -65,6 +65,7 @@ export default class extends React.Component { error: null, imgError: false, imgLoaded: false, + loadedImageDimensions: null, hover: false, }; }