Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove body pending and 2000 body limit #1458

Merged
merged 1 commit into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions js/models/messages.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ interface MessageAttributes {
expirationTimerUpdate: any;
unread: boolean;
group: any;
bodyPending: boolean;
timestamp: number;
status: MessageDeliveryStatus;
}
Expand All @@ -50,7 +49,6 @@ export interface MessageRegularProps {
isAdmin?: boolean;
weAreAdmin?: boolean;
text?: string;
bodyPending?: boolean;
id: string;
collapseMetadata?: boolean;
direction: 'incoming' | 'outgoing';
Expand Down
40 changes: 3 additions & 37 deletions js/models/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
loadQuoteData,
loadPreviewData,
} = window.Signal.Migrations;
const { bytesFromString } = window.Signal.Crypto;

window.AccountCache = Object.create(null);
window.AccountJobs = Object.create(null);
Expand Down Expand Up @@ -525,7 +524,6 @@

return {
text: this.createNonBreakingLastSeparator(this.get('body')),
bodyPending: this.get('bodyPending'),
id: this.id,
direction: this.isIncoming() ? 'incoming' : 'outgoing',
timestamp: this.get('sent_at'),
Expand Down Expand Up @@ -870,17 +868,10 @@
// TODO: In the future it might be best if we cache the upload results if possible.
// This way we don't upload duplicated data.

const attachmentsWithData = await Promise.all(
const finalAttachments = await Promise.all(
(this.get('attachments') || []).map(loadAttachmentData)
);
const {
body,
attachments: finalAttachments,
} = Whisper.Message.getLongMessageAttachment({
body: this.get('body'),
attachments: attachmentsWithData,
now: this.get('sent_at'),
});

const filenameOverridenAttachments = finalAttachments.map(attachment => ({
...attachment,
fileName: Signal.Types.Attachment.getSuggestedFilenameSending({
Expand All @@ -906,7 +897,7 @@
]);

return {
body,
body: this.get('body'),
attachments,
preview,
quote,
Expand Down Expand Up @@ -1531,31 +1522,6 @@
},
});

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

Whisper.Message.getLongMessageAttachment = ({ body, attachments, now }) => {
if (body.length <= 2048) {
return {
body,
attachments,
};
}

const data = bytesFromString(body);
const attachment = {
contentType: Whisper.Message.LONG_MESSAGE_CONTENT_TYPE,
fileName: `long-message-${now}.txt`,
data,
size: data.byteLength,
};

return {
body: body.slice(0, 2048),
attachments: [attachment, ...attachments],
};
};

Whisper.Message.refreshExpirationTimer = () =>
Whisper.ExpiringMessagesListener.update();

Expand Down
1 change: 0 additions & 1 deletion js/modules/attachment_downloads.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ async function _addAttachmentToMessage(message, attachment, { type, index }) {
const { data } = await Signal.Migrations.loadAttachmentData(attachment);
message.set({
body: attachment.isError ? message.get('body') : stringFromBytes(data),
bodyPending: false,
});
} finally {
Signal.Migrations.deleteAttachmentData(attachment.path);
Expand Down
2 changes: 0 additions & 2 deletions ts/components/conversation/Message.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,6 @@ class MessageInner extends React.PureComponent<MessageRegularProps, State> {
public renderText() {
const {
text,
bodyPending,
direction,
status,
conversationType,
Expand Down Expand Up @@ -578,7 +577,6 @@ class MessageInner extends React.PureComponent<MessageRegularProps, State> {
<MessageBody
text={contents || ''}
i18n={window.i18n}
bodyPending={bodyPending}
isGroup={conversationType === 'group'}
convoId={convoId}
disableLinks={multiSelectMode}
Expand Down
54 changes: 6 additions & 48 deletions ts/components/conversation/MessageBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { LocalizerType, RenderTextCallbackType } from '../../types/Util';

interface Props {
text: string;
bodyPending?: boolean;
/** If set, all emoji will be the same size. Otherwise, just one emoji will be large. */
disableJumbomoji?: boolean;
/** If set, links will be left alone instead of turned into clickable `<a>` tags. */
Expand Down Expand Up @@ -83,50 +82,26 @@ export class MessageBody extends React.Component<Props> {
isGroup: false,
};

public addDownloading(jsx: JSX.Element): JSX.Element {
const { i18n, bodyPending } = this.props;

return (
<span className="text-selectable">
{jsx}
{bodyPending ? (
<span className="module-message-body__highlight">
{' '}
{i18n('downloading')}
</span>
) : null}
</span>
);
public renderJsxSelectable(jsx: JSX.Element): JSX.Element {
return <span className="text-selectable">{jsx}</span>;
}

public render() {
const {
text,
bodyPending,
disableJumbomoji,
disableLinks,
i18n,
isGroup,
convoId,
} = this.props;
const sizeClass = disableJumbomoji ? undefined : getSizeClass(text);
const textWithPending = bodyPending ? `${text}...` : text;

const emoji = renderEmoji({
i18n,
text: textWithPending,
sizeClass,
key: 0,
renderNonEmoji: renderNewLines,
isGroup,
convoId,
});

if (disableLinks) {
return this.addDownloading(
return this.renderJsxSelectable(
renderEmoji({
i18n,
text: textWithPending,
text,
sizeClass,
key: 0,
renderNonEmoji: renderNewLines,
Expand All @@ -136,26 +111,9 @@ export class MessageBody extends React.Component<Props> {
);
}

const bodyContents = this.addDownloading(
<Linkify
text={textWithPending}
renderNonLink={({ key, text: nonLinkText }) => {
return renderEmoji({
i18n,
text: nonLinkText,
sizeClass,
key,
renderNonEmoji: renderNewLines,
isGroup,
convoId,
});
}}
/>
);

return this.addDownloading(
return this.renderJsxSelectable(
<Linkify
text={textWithPending}
text={text}
renderNonLink={({ key, text: nonLinkText }) => {
return renderEmoji({
i18n,
Expand Down
6 changes: 1 addition & 5 deletions ts/components/conversation/message/MessageMetadata.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ type Props = {
isAdmin?: boolean;
isDeletable: boolean;
text?: string;
bodyPending?: boolean;
id: string;
collapseMetadata?: boolean;
direction: 'incoming' | 'outgoing';
Expand Down Expand Up @@ -69,7 +68,6 @@ export const MessageMetadata = (props: Props) => {
expirationTimestamp,
status,
text,
bodyPending,
timestamp,
serverTimestamp,
isShowingImage,
Expand All @@ -86,7 +84,7 @@ export const MessageMetadata = (props: Props) => {
const withImageNoCaption = Boolean(!text && isShowingImage);
const showError = status === 'error' && isOutgoing;

const showStatus = Boolean(!bodyPending && status?.length && isOutgoing);
const showStatus = Boolean(status?.length && isOutgoing);
const messageStatusColor = withImageNoCaption
? 'white'
: props.theme.colors.sentMessageText;
Expand Down Expand Up @@ -124,8 +122,6 @@ export const MessageMetadata = (props: Props) => {
/>
) : null}
<MetadataSpacer />
{bodyPending ? <Spinner size="mini" direction={direction} /> : null}
<MetadataSpacer />
{showStatus ? (
<OutgoingMessageStatus
iconColor={messageStatusColor}
Expand Down
27 changes: 5 additions & 22 deletions ts/components/session/conversation/SessionCompositionBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ export class SessionCompositionBox extends React.Component<Props, State> {
spellCheck={true}
inputRef={this.textarea}
disabled={!typingEnabled}
maxLength={Constants.CONVERSATION.MAX_MESSAGE_BODY_LENGTH}
// maxLength={Constants.CONVERSATION.MAX_MESSAGE_BODY_LENGTH}
rows={1}
style={sendMessageStyle}
suggestionsPortalHost={this.container}
Expand Down Expand Up @@ -751,25 +751,8 @@ export class SessionCompositionBox extends React.Component<Props, State> {

// tslint:disable-next-line: cyclomatic-complexity
private async onSendMessage() {
const toUnicode = (str: string) => {
return str
.split('')
.map(value => {
const temp = value
.charCodeAt(0)
.toString(16)
.toUpperCase();
if (temp.length > 2) {
return `\\u${temp}`;
}
return value;
})
.join('');
};

// this is dirty but we have to replace all @(xxx) by @xxx manually here
const cleanMentions = (text: string): string => {
const textUnicode = toUnicode(text);
const matches = text.match(this.mentionsRegex);
let replacedMentions = text;
(matches || []).forEach(match => {
Expand Down Expand Up @@ -808,10 +791,10 @@ export class SessionCompositionBox extends React.Component<Props, State> {
}
// Verify message length
const msgLen = messagePlaintext?.length || 0;
if (msgLen > Constants.CONVERSATION.MAX_MESSAGE_BODY_LENGTH) {
ToastUtils.pushMessageBodyTooLong();
return;
}
// if (msgLen > Constants.CONVERSATION.MAX_MESSAGE_BODY_LENGTH) {
// ToastUtils.pushMessageBodyTooLong();
// return;
// }
if (msgLen === 0 && this.props.stagedAttachments?.length === 0) {
ToastUtils.pushMessageBodyMissing();
return;
Expand Down
36 changes: 1 addition & 35 deletions ts/receiver/attachments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,32 +75,6 @@ export async function downloadAttachment(attachment: any) {
};
}

async function processLongAttachments(
message: MessageModel,
attachments: Array<any>
): Promise<boolean> {
if (attachments.length === 0) {
return false;
}

if (attachments.length > 1) {
window.log.error(
`Received more than one long message attachment in message ${message.idForLogging()}`
);
}

const attachment = attachments[0];

message.set({ bodyPending: true });
await window.Signal.AttachmentDownloads.addJob(attachment, {
messageId: message.id,
type: 'long-message',
index: 0,
});

return true;
}

async function processNormalAttachments(
message: MessageModel,
normalAttachments: Array<any>
Expand Down Expand Up @@ -247,15 +221,7 @@ export async function queueAttachmentDownloads(

let count = 0;

const [longMessageAttachments, normalAttachments] = _.partition(
message.get('attachments') || [],
(attachment: any) =>
attachment.contentType === Whisper.Message.LONG_MESSAGE_CONTENT_TYPE
);

if (await processLongAttachments(message, longMessageAttachments)) {
count += 1;
}
const normalAttachments = message.get('attachments') || [];

count += await processNormalAttachments(message, normalAttachments);

Expand Down
2 changes: 1 addition & 1 deletion ts/session/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const TTL_DEFAULT = {

// User Interface
export const CONVERSATION = {
MAX_MESSAGE_BODY_LENGTH: 2000,
// MAX_MESSAGE_BODY_LENGTH: 2000,
DEFAULT_MEDIA_FETCH_COUNT: 50,
DEFAULT_DOCUMENTS_FETCH_COUNT: 150,
DEFAULT_MESSAGE_FETCH_COUNT: 30,
Expand Down