Hide long message attachments in quotes

This commit is contained in:
Evan Hahn 2021-03-25 13:36:50 -05:00 committed by GitHub
parent 6f404648d7
commit afe135df0c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 76 additions and 42 deletions

View file

@ -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}

View file

@ -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';
@ -62,13 +68,13 @@ const defaultMessageProps: MessagesProps = {
};
const renderInMessage = ({
attachment,
authorColor,
authorName,
authorPhoneNumber,
authorProfileName,
authorTitle,
isFromMe,
rawAttachment,
referencedMessageNotFound,
text: quoteText,
}: Props) => {
@ -76,7 +82,6 @@ const renderInMessage = ({
...defaultMessageProps,
authorColor,
quote: {
attachment,
authorId: 'an-author',
authorColor,
authorName,
@ -84,6 +89,7 @@ const renderInMessage = ({
authorProfileName,
authorTitle,
isFromMe,
rawAttachment,
referencedMessageNotFound,
sentAt: Date.now() - 30 * 1000,
text: quoteText,
@ -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(
@ -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
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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,
@ -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;

View file

@ -15,7 +15,6 @@ import { ContactName } from './ContactName';
import { getTextWithMentions } from '../../util/getTextWithMentions';
export type Props = {
attachment?: QuotedAttachmentType;
authorTitle: string;
authorPhoneNumber?: string;
authorProfileName?: string;
@ -29,6 +28,7 @@ export type Props = {
onClick?: () => void;
onClose?: () => void;
text: string;
rawAttachment?: QuotedAttachmentType;
referencedMessageNotFound: boolean;
};
@ -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;
@ -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;
@ -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;
@ -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
@ -253,6 +264,8 @@ export class Quote extends React.Component<Props, State> {
);
}
const attachment = getAttachment(rawAttachment);
if (!attachment) {
return null;
}

View file

@ -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
@ -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) {
@ -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) {
@ -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) {
@ -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,
@ -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,

View file

@ -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'));
});
});
});

View file

@ -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;

View file

@ -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."
@ -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."