Skip to content

Commit

Permalink
Hide long message attachments in quotes
Browse files Browse the repository at this point in the history
  • Loading branch information
EvanHahn-Signal committed Mar 25, 2021
1 parent 6f40464 commit afe135d
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 42 deletions.
2 changes: 1 addition & 1 deletion ts/components/conversation/Message.tsx
Expand Up @@ -1030,7 +1030,7 @@ export class Message extends React.PureComponent<Props, State> {
i18n={i18n}
onClick={clickHandler}
text={quote.text}
attachment={quote.attachment}
rawAttachment={quote.attachment}
isIncoming={direction === 'incoming'}
authorPhoneNumber={quote.authorPhoneNumber}
authorProfileName={quote.authorProfileName}
Expand Down
50 changes: 34 additions & 16 deletions ts/components/conversation/Quote.stories.tsx
Expand Up @@ -10,7 +10,13 @@ import { storiesOf } from '@storybook/react';
import { Colors } from '../../types/Colors';
import { pngUrl } from '../../storybook/Fixtures';
import { Message, Props as MessagesProps } from './Message';
import { AUDIO_MP3, IMAGE_PNG, MIMEType, VIDEO_MP4 } from '../../types/MIME';
import {
AUDIO_MP3,
IMAGE_PNG,
LONG_MESSAGE,
MIMEType,
VIDEO_MP4,
} from '../../types/MIME';
import { Props, Quote } from './Quote';
import { setup as setupI18n } from '../../../js/modules/i18n';
import enMessages from '../../../_locales/en/messages.json';
Expand Down Expand Up @@ -62,28 +68,28 @@ const defaultMessageProps: MessagesProps = {
};

const renderInMessage = ({
attachment,
authorColor,
authorName,
authorPhoneNumber,
authorProfileName,
authorTitle,
isFromMe,
rawAttachment,
referencedMessageNotFound,
text: quoteText,
}: Props) => {
const messageProps = {
...defaultMessageProps,
authorColor,
quote: {
attachment,
authorId: 'an-author',
authorColor,
authorName,
authorPhoneNumber,
authorProfileName,
authorTitle,
isFromMe,
rawAttachment,
referencedMessageNotFound,
sentAt: Date.now() - 30 * 1000,
text: quoteText,
Expand All @@ -100,7 +106,6 @@ const renderInMessage = ({
};

const createProps = (overrideProps: Partial<Props> = {}): Props => ({
attachment: overrideProps.attachment || undefined,
authorColor: overrideProps.authorColor || 'green',
authorName: text('authorName', overrideProps.authorName || ''),
authorPhoneNumber: text(
Expand All @@ -117,6 +122,7 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
isIncoming: boolean('isIncoming', overrideProps.isIncoming || false),
onClick: action('onClick'),
onClose: action('onClose'),
rawAttachment: overrideProps.rawAttachment || undefined,
referencedMessageNotFound: boolean(
'referencedMessageNotFound',
overrideProps.referencedMessageNotFound || false
Expand Down Expand Up @@ -186,7 +192,7 @@ story.add('Content Above', () => {

story.add('Image Only', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: IMAGE_PNG,
fileName: 'sax.png',
isVoiceMessage: false,
Expand All @@ -203,7 +209,7 @@ story.add('Image Only', () => {
});
story.add('Image Attachment', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: IMAGE_PNG,
fileName: 'sax.png',
isVoiceMessage: false,
Expand All @@ -219,7 +225,7 @@ story.add('Image Attachment', () => {

story.add('Image Attachment w/o Thumbnail', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: IMAGE_PNG,
fileName: 'sax.png',
isVoiceMessage: false,
Expand All @@ -231,7 +237,7 @@ story.add('Image Attachment w/o Thumbnail', () => {

story.add('Video Only', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: VIDEO_MP4,
fileName: 'great-video.mp4',
isVoiceMessage: false,
Expand All @@ -249,7 +255,7 @@ story.add('Video Only', () => {

story.add('Video Attachment', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: VIDEO_MP4,
fileName: 'great-video.mp4',
isVoiceMessage: false,
Expand All @@ -265,7 +271,7 @@ story.add('Video Attachment', () => {

story.add('Video Attachment w/o Thumbnail', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: VIDEO_MP4,
fileName: 'great-video.mp4',
isVoiceMessage: false,
Expand All @@ -277,7 +283,7 @@ story.add('Video Attachment w/o Thumbnail', () => {

story.add('Audio Only', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: AUDIO_MP3,
fileName: 'great-video.mp3',
isVoiceMessage: false,
Expand All @@ -291,7 +297,7 @@ story.add('Audio Only', () => {

story.add('Audio Attachment', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: AUDIO_MP3,
fileName: 'great-video.mp3',
isVoiceMessage: false,
Expand All @@ -303,7 +309,7 @@ story.add('Audio Attachment', () => {

story.add('Voice Message Only', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: AUDIO_MP3,
fileName: 'great-video.mp3',
isVoiceMessage: true,
Expand All @@ -317,7 +323,7 @@ story.add('Voice Message Only', () => {

story.add('Voice Message Attachment', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: AUDIO_MP3,
fileName: 'great-video.mp3',
isVoiceMessage: true,
Expand All @@ -329,7 +335,7 @@ story.add('Voice Message Attachment', () => {

story.add('Other File Only', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: 'application/json' as MIMEType,
fileName: 'great-data.json',
isVoiceMessage: false,
Expand All @@ -343,7 +349,7 @@ story.add('Other File Only', () => {

story.add('Other File Attachment', () => {
const props = createProps({
attachment: {
rawAttachment: {
contentType: 'application/json' as MIMEType,
fileName: 'great-data.json',
isVoiceMessage: false,
Expand All @@ -353,6 +359,18 @@ story.add('Other File Attachment', () => {
return <Quote {...props} />;
});

story.add('Long message attachment (should be hidden)', () => {
const props = createProps({
rawAttachment: {
contentType: LONG_MESSAGE,
fileName: 'signal-long-message-123.txt',
isVoiceMessage: false,
},
});

return <Quote {...props} />;
});

story.add('No Close Button', () => {
const props = createProps();
props.onClose = undefined;
Expand Down
23 changes: 18 additions & 5 deletions ts/components/conversation/Quote.tsx
Expand Up @@ -15,7 +15,6 @@ import { ContactName } from './ContactName';
import { getTextWithMentions } from '../../util/getTextWithMentions';

export type Props = {
attachment?: QuotedAttachmentType;
authorTitle: string;
authorPhoneNumber?: string;
authorProfileName?: string;
Expand All @@ -29,6 +28,7 @@ export type Props = {
onClick?: () => void;
onClose?: () => void;
text: string;
rawAttachment?: QuotedAttachmentType;
referencedMessageNotFound: boolean;
};

Expand All @@ -55,13 +55,22 @@ function validateQuote(quote: Props): boolean {
return true;
}

if (quote.attachment) {
if (quote.rawAttachment) {
return true;
}

return false;
}

// Long message attachments should not be shown.
function getAttachment(
rawAttachment: undefined | QuotedAttachmentType
): undefined | QuotedAttachmentType {
return rawAttachment && !MIME.isLongMessage(rawAttachment.contentType)
? rawAttachment
: undefined;
}

function getObjectUrl(thumbnail: Attachment | undefined): string | undefined {
if (thumbnail && thumbnail.objectUrl) {
return thumbnail.objectUrl;
Expand Down Expand Up @@ -173,7 +182,8 @@ export class Quote extends React.Component<Props, State> {
}

public renderGenericFile(): JSX.Element | null {
const { attachment, isIncoming } = this.props;
const { rawAttachment, isIncoming } = this.props;
const attachment = getAttachment(rawAttachment);

if (!attachment) {
return null;
Expand Down Expand Up @@ -205,8 +215,9 @@ export class Quote extends React.Component<Props, State> {
}

public renderIconContainer(): JSX.Element | null {
const { attachment } = this.props;
const { rawAttachment } = this.props;
const { imageBroken } = this.state;
const attachment = getAttachment(rawAttachment);

if (!attachment) {
return null;
Expand All @@ -233,7 +244,7 @@ export class Quote extends React.Component<Props, State> {
}

public renderText(): JSX.Element | null {
const { bodyRanges, i18n, text, attachment, isIncoming } = this.props;
const { bodyRanges, i18n, text, rawAttachment, isIncoming } = this.props;

if (text) {
const quoteText = bodyRanges
Expand All @@ -253,6 +264,8 @@ export class Quote extends React.Component<Props, State> {
);
}

const attachment = getAttachment(rawAttachment);

if (!attachment) {
return null;
}
Expand Down
26 changes: 8 additions & 18 deletions ts/models/messages.ts
Expand Up @@ -132,8 +132,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
attachment: typeof window.WhatIsThis
) => typeof window.WhatIsThis;

static LONG_MESSAGE_CONTENT_TYPE: string;

CURRENT_PROTOCOL_VERSION?: number;

// Set when sending some sync messages, so we get the functionality of
Expand Down Expand Up @@ -2580,10 +2578,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
this.get('attachments') || [];

const hasLongMessageAttachments = attachments.some(attachment => {
return (
attachment.contentType ===
window.Whisper.Message.LONG_MESSAGE_CONTENT_TYPE
);
return MIME.isLongMessage(attachment.contentType);
});

if (hasLongMessageAttachments) {
Expand All @@ -2605,9 +2600,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {

const [longMessageAttachments, normalAttachments] = _.partition(
attachments,
attachment =>
attachment.contentType ===
window.Whisper.Message.LONG_MESSAGE_CONTENT_TYPE
attachment => MIME.isLongMessage(attachment.contentType)
);

if (longMessageAttachments.length > 0) {
Expand Down Expand Up @@ -2698,11 +2691,11 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
} attachment downloads for message ${this.idForLogging()}`
);

const [longMessageAttachments, normalAttachments] = _.partition(
attachmentsToQueue,
attachment =>
attachment.contentType ===
window.Whisper.Message.LONG_MESSAGE_CONTENT_TYPE
const [
longMessageAttachments,
normalAttachments,
] = _.partition(attachmentsToQueue, attachment =>
MIME.isLongMessage(attachment.contentType)
);

if (longMessageAttachments.length > 1) {
Expand Down Expand Up @@ -3975,9 +3968,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {

window.Whisper.Message = MessageModel as typeof window.WhatIsThis;

// Receive will be enabled before we enable send
window.Whisper.Message.LONG_MESSAGE_CONTENT_TYPE = 'text/x-signal-plain';

window.Whisper.Message.getLongMessageAttachment = ({
body,
attachments,
Expand All @@ -3992,7 +3982,7 @@ window.Whisper.Message.getLongMessageAttachment = ({

const data = bytesFromString(body);
const attachment = {
contentType: window.Whisper.Message.LONG_MESSAGE_CONTENT_TYPE,
contentType: MIME.LONG_MESSAGE,
fileName: `long-message-${now}.txt`,
data,
size: data.byteLength,
Expand Down
11 changes: 11 additions & 0 deletions ts/test-node/types/MIME_test.ts
Expand Up @@ -16,4 +16,15 @@ describe('MIME', () => {
assert.isFalse(MIME.isGif('text/plain'));
});
});

describe('isLongMessage', () => {
it('returns true for long messages', () => {
assert.isTrue(MIME.isLongMessage('text/x-signal-plain'));
});

it('returns true for other content types', () => {
assert.isFalse(MIME.isLongMessage('text/plain'));
assert.isFalse(MIME.isLongMessage('image/gif'));
});
});
});
2 changes: 2 additions & 0 deletions ts/types/MIME.ts
Expand Up @@ -29,3 +29,5 @@ export const isVideo = (value: string): value is MIMEType =>
// recognize them as file attachments.
export const isAudio = (value: string): value is MIMEType =>
Boolean(value) && value.startsWith('audio/') && !value.endsWith('aiff');
export const isLongMessage = (value: unknown): value is MIMEType =>
value === LONG_MESSAGE;
4 changes: 2 additions & 2 deletions ts/util/lint/exceptions.json
Expand Up @@ -14685,7 +14685,7 @@
"rule": "React-useRef",
"path": "ts/components/conversation/Quote.js",
"line": " const imageRef = react_1.useRef(new Image());",
"lineNumber": 227,
"lineNumber": 236,
"reasonCategory": "usageTrusted",
"updated": "2021-01-20T21:30:08.430Z",
"reasonDetail": "Doesn't touch the DOM."
Expand All @@ -14694,7 +14694,7 @@
"rule": "React-useRef",
"path": "ts/components/conversation/Quote.tsx",
"line": " const imageRef = useRef(new Image());",
"lineNumber": 446,
"lineNumber": 459,
"reasonCategory": "usageTrusted",
"updated": "2021-01-20T21:30:08.430Z",
"reasonDetail": "Doesn't touch the DOM."
Expand Down

0 comments on commit afe135d

Please sign in to comment.