Fix timeline crash when deleting the oldest visible message

Co-authored-by: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com>
This commit is contained in:
automated-signal 2022-02-07 13:03:35 -08:00 committed by GitHub
parent dea9f269cc
commit d4de93e420
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 23 deletions

View File

@ -115,7 +115,7 @@ type PropsHousekeepingType = {
warning?: WarningType;
contactSpoofingReview?: ContactSpoofingReviewPropType;
getTimestampForMessage: (_: string) => number;
getTimestampForMessage: (messageId: string) => undefined | number;
getPreferredBadge: PreferredBadgeSelectorType;
i18n: LocalizerType;
theme: ThemeType;
@ -220,7 +220,7 @@ type StateType = {
oneTimeScrollRow?: number;
visibleRows?: {
newestFullyVisible?: VisibleRowType;
oldestPartiallyVisible?: VisibleRowType;
oldestPartiallyVisibleMessageId?: string;
oldestFullyVisible?: VisibleRowType;
};
@ -612,7 +612,7 @@ export class Timeline extends React.PureComponent<PropsType, StateType> {
}
let newestFullyVisible: undefined | VisibleRowType;
let oldestPartiallyVisible: undefined | VisibleRowType;
let oldestPartiallyVisibleMessageId: undefined | string;
let oldestFullyVisible: undefined | VisibleRowType;
const { children } = innerScrollContainer;
@ -646,20 +646,18 @@ export class Timeline extends React.PureComponent<PropsType, StateType> {
continue;
}
const thisRow = {
offsetTop,
row: parseInt(child.getAttribute('data-row') || '-1', 10),
id,
};
const bottom = offsetTop + offsetHeight;
if (bottom >= visibleTop && !oldestPartiallyVisible) {
oldestPartiallyVisible = thisRow;
if (bottom >= visibleTop && !oldestPartiallyVisibleMessageId) {
oldestPartiallyVisibleMessageId = id;
}
if (offsetTop + AT_TOP_THRESHOLD >= visibleTop) {
oldestFullyVisible = thisRow;
oldestFullyVisible = {
offsetTop,
row: parseInt(child.getAttribute('data-row') || '-1', 10),
id,
};
break;
}
}
@ -667,7 +665,7 @@ export class Timeline extends React.PureComponent<PropsType, StateType> {
this.setState(oldState => {
const visibleRows = {
newestFullyVisible,
oldestPartiallyVisible,
oldestPartiallyVisibleMessageId,
oldestFullyVisible,
};
@ -1289,8 +1287,15 @@ export class Timeline extends React.PureComponent<PropsType, StateType> {
}
let floatingHeader: ReactNode;
const oldestPartiallyVisibleRow = visibleRows?.oldestPartiallyVisible;
if (oldestPartiallyVisibleRow) {
const oldestPartiallyVisibleMessageId =
visibleRows?.oldestPartiallyVisibleMessageId;
// It's possible that a message was removed from `items` but we still have its ID in
// state. `getTimestampForMessage` might return undefined in that case.
const oldestPartiallyVisibleMessageTimestamp =
oldestPartiallyVisibleMessageId
? getTimestampForMessage(oldestPartiallyVisibleMessageId)
: undefined;
if (oldestPartiallyVisibleMessageTimestamp) {
floatingHeader = (
<TimelineFloatingHeader
i18n={i18n}
@ -1300,10 +1305,10 @@ export class Timeline extends React.PureComponent<PropsType, StateType> {
? { marginTop: lastMeasuredWarningHeight }
: undefined
}
timestamp={getTimestampForMessage(oldestPartiallyVisibleRow.id)}
timestamp={oldestPartiallyVisibleMessageTimestamp}
visible={
(hasRecentlyScrolled || isLoadingMessages) &&
(!haveOldest || oldestPartiallyVisibleRow.id !== items[0])
(!haveOldest || oldestPartiallyVisibleMessageId !== items[0])
}
/>
);

View File

@ -38,7 +38,7 @@ import { renderEmojiPicker } from './renderEmojiPicker';
import { renderReactionPicker } from './renderReactionPicker';
import { getOwn } from '../../util/getOwn';
import { assert, strictAssert } from '../../util/assert';
import { assert } from '../../util/assert';
import { missingCaseError } from '../../util/missingCaseError';
import { getGroupMemberships } from '../../util/getGroupMemberships';
import {
@ -295,11 +295,8 @@ const mapStateToProps = (state: StateType, props: ExternalProps) => {
const selectedMessage = getSelectedMessage(state);
const messageSelector = getMessageSelector(state);
const getTimestampForMessage = (messageId: string): number => {
const result = messageSelector(messageId)?.timestamp;
strictAssert(result, 'Expected a message');
return result;
};
const getTimestampForMessage = (messageId: string): undefined | number =>
messageSelector(messageId)?.timestamp;
return {
id,