From 20246c4d07951aadc8a98886705218d24db06d10 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 14:43:34 -0400 Subject: [PATCH 01/15] Classify all images and videos as visual media MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even if we can’t play it back. Handle that in the lightbox. Also: Exclude voice messages. --- ts/types/Attachment.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index e4e58ffe846..414bce856ab 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -1,7 +1,6 @@ import is from '@sindresorhus/is'; import moment from 'moment'; -import * as GoogleChrome from '../util/GoogleChrome'; import * as MIME from './MIME'; import { arrayBufferToObjectURL } from '../util/arrayBufferToObjectURL'; import { saveURLAsFile } from '../util/saveURLAsFile'; @@ -35,9 +34,11 @@ export const isVisualMedia = (attachment: Attachment): boolean => { return false; } - const isSupportedImageType = GoogleChrome.isImageTypeSupported(contentType); - const isSupportedVideoType = GoogleChrome.isVideoTypeSupported(contentType); - return isSupportedImageType || isSupportedVideoType; + if (isVoiceMessage(attachment)) { + return false; + } + + return MIME.isImage(contentType) || MIME.isVideo(contentType); }; export const isVoiceMessage = (attachment: Attachment): boolean => { From 53918d68de334362faf782288b7a87039f118a9d Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 15:24:46 -0400 Subject: [PATCH 02/15] Add `Attachment.isFile` definition --- ts/test/types/Attachment_test.ts | 39 ++++++++++++++++++++++++++++++++ ts/types/Attachment.ts | 18 +++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/ts/test/types/Attachment_test.ts b/ts/test/types/Attachment_test.ts index 14b0cdcee78..23fa4099509 100644 --- a/ts/test/types/Attachment_test.ts +++ b/ts/test/types/Attachment_test.ts @@ -59,6 +59,45 @@ describe('Attachment', () => { }); }); + describe('isFile', () => { + it('should return true for JSON', () => { + const attachment: Attachment.Attachment = { + fileName: 'foo.json', + data: stringToArrayBuffer('{"foo": "bar"}'), + contentType: MIME.APPLICATION_JSON, + }; + assert.isTrue(Attachment.isFile(attachment)); + }); + + it('should return false for images', () => { + const attachment: Attachment.Attachment = { + fileName: 'meme.gif', + data: stringToArrayBuffer('gif'), + contentType: MIME.IMAGE_GIF, + }; + assert.isFalse(Attachment.isFile(attachment)); + }); + + it('should return false for videos', () => { + const attachment: Attachment.Attachment = { + fileName: 'meme.mp4', + data: stringToArrayBuffer('mp4'), + contentType: MIME.VIDEO_MP4, + }; + assert.isFalse(Attachment.isFile(attachment)); + }); + + it('should return false for voice message attachment', () => { + const attachment: Attachment.Attachment = { + fileName: 'Voice Message.aac', + flags: SignalService.AttachmentPointer.Flags.VOICE_MESSAGE, + data: stringToArrayBuffer('voice message'), + contentType: MIME.AUDIO_AAC, + }; + assert.isFalse(Attachment.isFile(attachment)); + }); + }); + describe('isVoiceMessage', () => { it('should return true for voice message attachment', () => { const attachment: Attachment.Attachment = { diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index 414bce856ab..3599f184fdf 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -41,6 +41,24 @@ export const isVisualMedia = (attachment: Attachment): boolean => { return MIME.isImage(contentType) || MIME.isVideo(contentType); }; +export const isFile = (attachment: Attachment): boolean => { + const { contentType } = attachment; + + if (is.undefined(contentType)) { + return false; + } + + if (isVisualMedia(attachment)) { + return false; + } + + if (isVoiceMessage(attachment)) { + return false; + } + + return true; +}; + export const isVoiceMessage = (attachment: Attachment): boolean => { const flag = SignalService.AttachmentPointer.Flags.VOICE_MESSAGE; const hasFlag = From 63bd9dcc612c6dc1f77738e6ac4adc66ea4320ea Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 15:47:33 -0400 Subject: [PATCH 03/15] Add tests for `Attachment.isVisualMedia` --- ts/test/types/Attachment_test.ts | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/ts/test/types/Attachment_test.ts b/ts/test/types/Attachment_test.ts index 23fa4099509..b7ef5f75492 100644 --- a/ts/test/types/Attachment_test.ts +++ b/ts/test/types/Attachment_test.ts @@ -59,6 +59,45 @@ describe('Attachment', () => { }); }); + describe('isVisualMedia', () => { + it('should return true for images', () => { + const attachment: Attachment.Attachment = { + fileName: 'meme.gif', + data: stringToArrayBuffer('gif'), + contentType: MIME.IMAGE_GIF, + }; + assert.isTrue(Attachment.isVisualMedia(attachment)); + }); + + it('should return true for videos', () => { + const attachment: Attachment.Attachment = { + fileName: 'meme.mp4', + data: stringToArrayBuffer('mp4'), + contentType: MIME.VIDEO_MP4, + }; + assert.isTrue(Attachment.isVisualMedia(attachment)); + }); + + it('should return false for voice message attachment', () => { + const attachment: Attachment.Attachment = { + fileName: 'Voice Message.aac', + flags: SignalService.AttachmentPointer.Flags.VOICE_MESSAGE, + data: stringToArrayBuffer('voice message'), + contentType: MIME.AUDIO_AAC, + }; + assert.isFalse(Attachment.isVisualMedia(attachment)); + }); + + it('should return false for other attachments', () => { + const attachment: Attachment.Attachment = { + fileName: 'foo.json', + data: stringToArrayBuffer('{"foo": "bar"}'), + contentType: MIME.APPLICATION_JSON, + }; + assert.isFalse(Attachment.isVisualMedia(attachment)); + }); + }); + describe('isFile', () => { it('should return true for JSON', () => { const attachment: Attachment.Attachment = { From 101041f106efa273dec338bf22cc72b95963a1b7 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 15:48:48 -0400 Subject: [PATCH 04/15] Derive `Message.CURRENT_SCHEMA_VERSION` --- js/modules/types/message.js | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index f2d8288cbca..defefb4f373 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -35,13 +35,6 @@ const PRIVATE = 'private'; const INITIAL_SCHEMA_VERSION = 0; -// Increment this version number every time we add a message schema upgrade -// step. This will allow us to retroactively upgrade existing messages. As we -// add more upgrade steps, we could design a pipeline that does this -// incrementally, e.g. from version 0 / unknown -> 1, 1 --> 2, etc., similar to -// how we do database migrations: -exports.CURRENT_SCHEMA_VERSION = 6; - // Public API exports.GROUP = GROUP; exports.PRIVATE = PRIVATE; @@ -212,7 +205,6 @@ exports._mapQuotedAttachments = upgradeAttachment => async ( }; const toVersion0 = async message => exports.initializeSchemaVersion(message); - const toVersion1 = exports._withSchemaVersion( 1, exports._mapAttachments(Attachment.autoOrientJPEG) @@ -230,7 +222,6 @@ const toVersion4 = exports._withSchemaVersion( exports._mapQuotedAttachments(Attachment.migrateDataToFileSystem) ); const toVersion5 = exports._withSchemaVersion(5, initializeAttachmentMetadata); - const toVersion6 = exports._withSchemaVersion( 6, exports._mapContact( @@ -238,6 +229,16 @@ const toVersion6 = exports._withSchemaVersion( ) ); +const VERSIONS = [ + toVersion0, + toVersion1, + toVersion2, + toVersion3, + toVersion4, + toVersion5, +]; +exports.CURRENT_SCHEMA_VERSION = VERSIONS.length - 1; + // UpgradeStep exports.upgradeSchema = async (rawMessage, { writeNewAttachmentData } = {}) => { if (!isFunction(writeNewAttachmentData)) { @@ -245,18 +246,8 @@ exports.upgradeSchema = async (rawMessage, { writeNewAttachmentData } = {}) => { } let message = rawMessage; - const versions = [ - toVersion0, - toVersion1, - toVersion2, - toVersion3, - toVersion4, - toVersion5, - toVersion6, - ]; - - for (let i = 0, max = versions.length; i < max; i += 1) { - const currentVersion = versions[i]; + // eslint-disable-next-line no-restricted-syntax + for (const currentVersion of VERSIONS) { // We really do want this intra-loop await because this is a chained async action, // each step dependent on the previous // eslint-disable-next-line no-await-in-loop From f4a5bc9907f84640fb769147ab596ce6354ba204 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 15:49:06 -0400 Subject: [PATCH 05/15] Add new MIME types --- ts/types/MIME.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ts/types/MIME.ts b/ts/types/MIME.ts index c2cc66ac588..cc4c61b3ae2 100644 --- a/ts/types/MIME.ts +++ b/ts/types/MIME.ts @@ -1,10 +1,12 @@ export type MIMEType = string & { _mimeTypeBrand: any }; export const APPLICATION_OCTET_STREAM = 'application/octet-stream' as MIMEType; +export const APPLICATION_JSON = 'application/json' as MIMEType; export const AUDIO_AAC = 'audio/aac' as MIMEType; export const AUDIO_MP3 = 'audio/mp3' as MIMEType; export const IMAGE_GIF = 'image/gif' as MIMEType; export const IMAGE_JPEG = 'image/jpeg' as MIMEType; +export const VIDEO_MP4 = 'video/mp4' as MIMEType; export const VIDEO_QUICKTIME = 'video/quicktime' as MIMEType; export const isJPEG = (value: MIMEType): boolean => value === 'image/jpeg'; From 16bc1d34c6bf90ed09f4e5141d5cd11cc9231d80 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 15:50:39 -0400 Subject: [PATCH 06/15] Message schema 6: Change classification of media and documents For an easier implementation, we change our original definition of `initializeAttachmentMetadata`. This means we have to re-run it marked as version 6 and mark schema version 5 as deprecated as its definition has changed. --- js/modules/types/message.js | 16 +++- test/modules/types/message_test.js | 9 +- .../initializeAttachmentMetadata_test.ts | 87 ++++++++++++++++++- .../message/initializeAttachmentMetadata.ts | 23 ++--- 4 files changed, 120 insertions(+), 15 deletions(-) diff --git a/js/modules/types/message.js b/js/modules/types/message.js index defefb4f373..bb03f0d11bb 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -25,13 +25,21 @@ const PRIVATE = 'private'; // - Attachments: Write attachment data to disk and store relative path to it. // Version 4 // - Quotes: Write thumbnail data to disk and store relative path to it. -// Version 5 +// Version 5 (deprecated) // - Attachments: Track number and kind of attachments for media gallery // - `hasAttachments?: 1 | 0` // - `hasVisualMediaAttachments?: 1 | undefined` (for media gallery ‘Media’ view) // - `hasFileAttachments?: 1 | undefined` (for media gallery ‘Documents’ view) +// - IMPORTANT: Version 7 changes the classification of visual media and files. +// Therefore version 5 is considered deprecated. For an easier implementation, +// new files have the same classification in version 5 as in version 7. // Version 6 // - Contact: Write contact avatar to disk, ensure contact data is well-formed +// Version 7 (supersedes attachment classification in version 5) +// - Attachments: Update classification for: +// - `hasVisualMediaAttachments`: Include all images and video regardless of +// whether Chromium can render it or not. +// - `hasFileAttachments`: Exclude voice messages. const INITIAL_SCHEMA_VERSION = 0; @@ -228,6 +236,10 @@ const toVersion6 = exports._withSchemaVersion( Contact.parseAndWriteAvatar(Attachment.migrateDataToFileSystem) ) ); +// IMPORTANT: We’ve updated our definition of `initializeAttachmentMetadata`, so +// we need to run it again on existing items that have previously been incorrectly +// classified: +const toVersion7 = exports._withSchemaVersion(7, initializeAttachmentMetadata); const VERSIONS = [ toVersion0, @@ -236,6 +248,8 @@ const VERSIONS = [ toVersion3, toVersion4, toVersion5, + toVersion6, + toVersion7, ]; exports.CURRENT_SCHEMA_VERSION = VERSIONS.length - 1; diff --git a/test/modules/types/message_test.js b/test/modules/types/message_test.js index 2343bd803a6..61535c569c7 100644 --- a/test/modules/types/message_test.js +++ b/test/modules/types/message_test.js @@ -2,6 +2,7 @@ const { assert } = require('chai'); const sinon = require('sinon'); const Message = require('../../../js/modules/types/message'); +const { SignalService } = require('../../../ts/protobuf'); const { stringToArrayBuffer, } = require('../../../js/modules/string_to_array_buffer'); @@ -242,7 +243,8 @@ describe('Message', () => { const input = { attachments: [ { - contentType: 'application/json', + contentType: 'audio/aac', + flags: SignalService.AttachmentPointer.Flags.VOICE_MESSAGE, data: stringToArrayBuffer('It’s easy if you try'), fileName: 'test\u202Dfig.exe', size: 1111, @@ -253,7 +255,8 @@ describe('Message', () => { const expected = { attachments: [ { - contentType: 'application/json', + contentType: 'audio/aac', + flags: 1, path: 'abc/abcdefg', fileName: 'test\uFFFDfig.exe', size: 1111, @@ -261,7 +264,7 @@ describe('Message', () => { ], hasAttachments: 1, hasVisualMediaAttachments: undefined, - hasFileAttachments: 1, + hasFileAttachments: undefined, schemaVersion: Message.CURRENT_SCHEMA_VERSION, contact: [], }; diff --git a/ts/test/types/message/initializeAttachmentMetadata_test.ts b/ts/test/types/message/initializeAttachmentMetadata_test.ts index 9c87215f3bd..16ba687e3a0 100644 --- a/ts/test/types/message/initializeAttachmentMetadata_test.ts +++ b/ts/test/types/message/initializeAttachmentMetadata_test.ts @@ -3,13 +3,14 @@ import { assert } from 'chai'; import * as Message from '../../../../ts/types/message/initializeAttachmentMetadata'; import { IncomingMessage } from '../../../../ts/types/Message'; +import { SignalService } from '../../../../ts/protobuf'; import * as MIME from '../../../../ts/types/MIME'; // @ts-ignore import { stringToArrayBuffer } from '../../../../js/modules/string_to_array_buffer'; describe('Message', () => { describe('initializeAttachmentMetadata', () => { - it('should handle visual media attachments', async () => { + it('should classify visual media attachments', async () => { const input: IncomingMessage = { type: 'incoming', conversationId: 'foo', @@ -49,5 +50,89 @@ describe('Message', () => { const actual = await Message.initializeAttachmentMetadata(input); assert.deepEqual(actual, expected); }); + + it('should classify file attachments', async () => { + const input: IncomingMessage = { + type: 'incoming', + conversationId: 'foo', + id: '11111111-1111-1111-1111-111111111111', + timestamp: 1523317140899, + received_at: 1523317140899, + sent_at: 1523317140800, + attachments: [ + { + contentType: MIME.APPLICATION_OCTET_STREAM, + data: stringToArrayBuffer('foo'), + fileName: 'foo.bin', + size: 1111, + }, + ], + }; + const expected: IncomingMessage = { + type: 'incoming', + conversationId: 'foo', + id: '11111111-1111-1111-1111-111111111111', + timestamp: 1523317140899, + received_at: 1523317140899, + sent_at: 1523317140800, + attachments: [ + { + contentType: MIME.APPLICATION_OCTET_STREAM, + data: stringToArrayBuffer('foo'), + fileName: 'foo.bin', + size: 1111, + }, + ], + hasAttachments: 1, + hasVisualMediaAttachments: undefined, + hasFileAttachments: 1, + }; + + const actual = await Message.initializeAttachmentMetadata(input); + assert.deepEqual(actual, expected); + }); + + it('should classify voice message attachments', async () => { + const input: IncomingMessage = { + type: 'incoming', + conversationId: 'foo', + id: '11111111-1111-1111-1111-111111111111', + timestamp: 1523317140899, + received_at: 1523317140899, + sent_at: 1523317140800, + attachments: [ + { + contentType: MIME.AUDIO_AAC, + flags: SignalService.AttachmentPointer.Flags.VOICE_MESSAGE, + data: stringToArrayBuffer('foo'), + fileName: 'Voice Message.aac', + size: 1111, + }, + ], + }; + const expected: IncomingMessage = { + type: 'incoming', + conversationId: 'foo', + id: '11111111-1111-1111-1111-111111111111', + timestamp: 1523317140899, + received_at: 1523317140899, + sent_at: 1523317140800, + attachments: [ + { + contentType: MIME.AUDIO_AAC, + flags: SignalService.AttachmentPointer.Flags.VOICE_MESSAGE, + data: stringToArrayBuffer('foo'), + fileName: 'Voice Message.aac', + size: 1111, + }, + ], + hasAttachments: 1, + hasVisualMediaAttachments: undefined, + hasFileAttachments: undefined, + }; + + const actual = await Message.initializeAttachmentMetadata(input); + assert.deepEqual(actual, expected); + }); }); }); diff --git a/ts/types/message/initializeAttachmentMetadata.ts b/ts/types/message/initializeAttachmentMetadata.ts index 317dacb482f..3ab02722e4b 100644 --- a/ts/types/message/initializeAttachmentMetadata.ts +++ b/ts/types/message/initializeAttachmentMetadata.ts @@ -1,8 +1,14 @@ -import { partition } from 'lodash'; - import * as Attachment from '../Attachment'; import * as IndexedDB from '../IndexedDB'; -import { Message } from '../Message'; +import { Message, UserMessage } from '../Message'; + +const hasAttachment = ( + predicate: (value: Attachment.Attachment) => boolean +) => (message: UserMessage): IndexedDB.IndexablePresence => + IndexedDB.toIndexablePresence(message.attachments.some(predicate)); + +const hasFileAttachment = hasAttachment(Attachment.isFile); +const hasVisualMediaAttachment = hasAttachment(Attachment.isVisualMedia); export const initializeAttachmentMetadata = async ( message: Message @@ -14,17 +20,14 @@ export const initializeAttachmentMetadata = async ( const hasAttachments = IndexedDB.toIndexableBoolean( message.attachments.length > 0 ); - const [hasVisualMediaAttachments, hasFileAttachments] = partition( - message.attachments, - Attachment.isVisualMedia - ) - .map(attachments => attachments.length > 0) - .map(IndexedDB.toIndexablePresence); + + const hasFileAttachments = hasFileAttachment(message); + const hasVisualMediaAttachments = hasVisualMediaAttachment(message); return { ...message, hasAttachments, - hasVisualMediaAttachments, hasFileAttachments, + hasVisualMediaAttachments, }; }; From 623bdd92841f3ad7bc33729e06a82e1ee4a562f2 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 21:20:06 -0400 Subject: [PATCH 07/15] Port `colorSVG` from Sass to TypeScript (React) --- ts/styles/colorSVG.ts | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 ts/styles/colorSVG.ts diff --git a/ts/styles/colorSVG.ts b/ts/styles/colorSVG.ts new file mode 100644 index 00000000000..a749e5fdc70 --- /dev/null +++ b/ts/styles/colorSVG.ts @@ -0,0 +1,7 @@ +export const colorSVG = (url: string, color: string) => { + return { + WebkitMask: `url(${url}) no-repeat center`, + WebkitMaskSize: '100%', + backgroundColor: color, + }; +}; From 90329a2764d48e2430a217b6331732a3cc4a41e8 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Mon, 7 May 2018 21:20:39 -0400 Subject: [PATCH 08/15] Display icon for unsupported file formats Still allows users to download media. --- ts/components/Lightbox.md | 50 +++++++++++++++++++++++++++++ ts/components/Lightbox.tsx | 57 +++++++++++++++++++++++++++++----- ts/components/styles/Colors.ts | 1 + 3 files changed, 101 insertions(+), 7 deletions(-) diff --git a/ts/components/Lightbox.md b/ts/components/Lightbox.md index 079dc642cbd..0840244f1a3 100644 --- a/ts/components/Lightbox.md +++ b/ts/components/Lightbox.md @@ -1,3 +1,5 @@ +## Image (supported format) + ```js const noop = () => {}; @@ -9,3 +11,51 @@ const noop = () => {}; /> ; ``` + +## Image (unsupported format) + +```js +const noop = () => {}; + +
+ +
; +``` + +## Video (supported format) + +```js +const noop = () => {}; + +
+ +
; +``` + +## Video (unsupported format) + +```js +const noop = () => {}; + +
+ +
; +``` + +## Unsupported file format + +```js +const noop = () => {}; + +
+ +
; +``` diff --git a/ts/components/Lightbox.tsx b/ts/components/Lightbox.tsx index af91914888d..23ca8a6b8e8 100644 --- a/ts/components/Lightbox.tsx +++ b/ts/components/Lightbox.tsx @@ -3,8 +3,10 @@ import React from 'react'; import classNames from 'classnames'; import is from '@sindresorhus/is'; +import * as Colors from './styles/Colors'; import * as GoogleChrome from '../util/GoogleChrome'; import * as MIME from '../types/MIME'; +import { colorSVG } from '../styles/colorSVG'; interface Props { close: () => void; @@ -50,6 +52,13 @@ const styles = { maxHeight: '100%', objectFit: 'contain', } as React.CSSProperties, + video: { + flexGrow: 1, + flexShrink: 0, + maxWidth: '100%', + maxHeight: '100%', + objectFit: 'contain', + } as React.CSSProperties, controlsOffsetPlaceholder: { width: CONTROLS_WIDTH, marginRight: CONTROLS_SPACING, @@ -110,6 +119,25 @@ const IconButtonPlaceholder = () => (
); +const Icon = ({ + onClick, + url, +}: { + onClick?: ( + event: React.MouseEvent + ) => void; + url: string; +}) => ( +
+); + export class Lightbox extends React.Component { private containerRef: HTMLDivElement | null = null; @@ -172,8 +200,8 @@ export class Lightbox extends React.Component { objectURL: string; contentType: MIME.MIMEType; }) => { - const isImage = GoogleChrome.isImageTypeSupported(contentType); - if (isImage) { + const isImageTypeSupported = GoogleChrome.isImageTypeSupported(contentType); + if (isImageTypeSupported) { return ( { ); } - const isVideo = GoogleChrome.isVideoTypeSupported(contentType); - if (isVideo) { + const isVideoTypeSupported = GoogleChrome.isVideoTypeSupported(contentType); + if (isVideoTypeSupported) { return ( -