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

fix: ensure processing of markup in comment #24319

Merged
merged 11 commits into from
Sep 21, 2023
4 changes: 2 additions & 2 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -1272,9 +1272,9 @@ const CONST = {
},

// Auth limit is 60k for the column but we store edits and other metadata along the html so let's use a lower limit to accommodate for it.
MAX_COMMENT_LENGTH: 15000,
MAX_COMMENT_LENGTH: 10000,

// Furthermore, applying markup is very resource-consuming, so let's set a slightly lower limit for that
// Use the same value as MAX_COMMENT_LENGTH to ensure the entire comment is parsed. Note that applying markup is very resource-consuming.
MAX_MARKUP_LENGTH: 10000,

MAX_THREAD_REPLIES_PREVIEW: 99,
Expand Down
8 changes: 7 additions & 1 deletion src/components/ExceededCommentLength.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
import {debounce} from 'lodash';
import CONST from '../CONST';
import * as ReportUtils from '../libs/ReportUtils';
import useLocalize from '../hooks/useLocalize';
import Text from './Text';
import styles from '../styles/styles';

Expand All @@ -15,6 +16,7 @@ const propTypes = {
};

function ExceededCommentLength(props) {
const {numberFormat, translate} = useLocalize();
const [commentLength, setCommentLength] = useState(0);
const updateCommentLength = useMemo(
() =>
Expand All @@ -34,7 +36,11 @@ function ExceededCommentLength(props) {
return null;
}

return <Text style={[styles.textMicro, styles.textDanger, styles.chatItemComposeSecondaryRow, styles.mlAuto, styles.pl2]}>{`${commentLength}/${CONST.MAX_COMMENT_LENGTH}`}</Text>;
return (
<Text style={[styles.textMicro, styles.textDanger, styles.chatItemComposeSecondaryRow, styles.mlAuto, styles.pl2]}>
{translate('composer.commentExceededMaxLength', {formattedMaxLength: numberFormat(CONST.MAX_COMMENT_LENGTH)})}
</Text>
);
}

ExceededCommentLength.propTypes = propTypes;
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export default {
composer: {
noExtensionFoundForMimeType: 'No extension found for mime type',
problemGettingImageYouPasted: 'There was a problem getting the image you pasted',
commentExceededMaxLength: ({formattedMaxLength}) => `The maximum comment length is ${formattedMaxLength} characters.`,
},
baseUpdateAppModal: {
updateApp: 'Update app',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ export default {
composer: {
noExtensionFoundForMimeType: 'No se encontró una extension para este tipo de contenido',
problemGettingImageYouPasted: 'Ha ocurrido un problema al obtener la imagen que has pegado',
commentExceededMaxLength: ({formattedMaxLength}) => `El comentario debe tener máximo ${formattedMaxLength} caracteres.`,
},
baseUpdateAppModal: {
updateApp: 'Actualizar app',
Expand Down
4 changes: 2 additions & 2 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1485,15 +1485,15 @@ function hasReportNameError(report) {
}

/**
* For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
* For comments shorter than or equal to 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
* For longer comments, skip parsing, but still escape the text, and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!!
*
* @param {String} text
* @returns {String}
*/
function getParsedComment(text) {
const parser = new ExpensiMark();
return text.length < CONST.MAX_MARKUP_LENGTH ? parser.replace(text) : _.escape(text);
return text.length <= CONST.MAX_MARKUP_LENGTH ? parser.replace(text) : _.escape(text);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ const removeLinksFromHtml = (html, links) => {
*/
const handleUserDeletedLinksInHtml = (newCommentText, originalHtml) => {
const parser = new ExpensiMark();
if (newCommentText.length >= CONST.MAX_MARKUP_LENGTH) {
if (newCommentText.length > CONST.MAX_MARKUP_LENGTH) {
return newCommentText;
}
const markdownOriginalComment = parser.htmlToMarkdown(originalHtml).trim();
Expand All @@ -1043,10 +1043,10 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
const htmlForNewComment = handleUserDeletedLinksInHtml(textForNewComment, originalCommentHTML);
const reportComment = parser.htmlToText(htmlForNewComment);

// For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
// For comments shorter than or equal to 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
// For longer comments, skip parsing and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!!
let parsedOriginalCommentHTML = originalCommentHTML;
if (textForNewComment.length < CONST.MAX_MARKUP_LENGTH) {
if (textForNewComment.length <= CONST.MAX_MARKUP_LENGTH) {
const autolinkFilter = {filterRules: _.filter(_.pluck(parser.rules, 'name'), (name) => name !== 'autolink')};
parsedOriginalCommentHTML = parser.replace(parser.htmlToMarkdown(originalCommentHTML).trim(), autolinkFilter);
}
Expand Down