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.

Sign up for GitHub

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

Jump to bottom

Fix/15131: Special html characters like &nbsp are not escaped in name and can bug out the UI #15340

Merged
merged 16 commits into from
Mar 28, 2023
Merged

Fix/15131: Special html characters like &nbsp are not escaped in name and can bug out the UI #15340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0290a79
fix: remove decode username
tienifr Feb 22, 2023
c93f6dc
fix: merge main
tienifr Feb 22, 2023
478f1aa
fix: decode IOU message
tienifr Feb 28, 2023
3726b22
fix: merge main
tienifr Feb 28, 2023
87d4689
fix: decode message in iou detail
tienifr Mar 1, 2023
b39a9f2
fix: merge main
tienifr Mar 1, 2023
6b7f5bc
replace parser.replace by htmlEncode and encode comment
tienifr Mar 2, 2023
5862120
merge branch 'main'
tienifr Mar 7, 2023
d9e7c27
merge main
tienifr Mar 7, 2023
c3adbb9
fix: decode html instead of text
tienifr Mar 7, 2023
01f255d
merge main
tienifr Mar 12, 2023
0c40bc1
Merge branch 'main'
tienifr Mar 15, 2023
7067ae8
fix: remove Str.htmlDecode
tienifr Mar 21, 2023
663e909
resolve conflict
tienifr Mar 27, 2023
6a0a7f9
remove htmlDecode applied on plaintext
tienifr Mar 27, 2023
335d87c
merge main
tienifr Mar 28, 2023
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
5 changes: 2 additions & 3 deletions src/components/ReportActionItem/IOUQuote.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import {View, Pressable} from 'react-native';
import PropTypes from 'prop-types';
import _ from 'underscore';
import Str from 'expensify-common/lib/str';
import Text from '../Text';
import Icon from '../Icon';
import * as Expensicons from '../Icon/Expensicons';
Expand Down Expand Up @@ -76,14 +75,14 @@ const IOUQuote = props => (
<Text style={[styles.flex1, styles.mr2]}>
<Text style={props.shouldAllowViewDetails && styles.chatItemMessageLink}>
{/* Get first word of IOU message */}
{Str.htmlDecode(fragment.text.split(' ')[0])}
{fragment.text.split(' ')[0]}
</Text>
<Text style={[styles.chatItemMessage, props.shouldAllowViewDetails
? styles.cursorPointer
: styles.cursorDefault]}
>
{/* Get remainder of IOU message */}
{Str.htmlDecode(fragment.text.substring(fragment.text.indexOf(' ')))}
{fragment.text.substring(fragment.text.indexOf(' '))}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes a regression on staging https://staging.new.expensify.com when cancelling a money request with IOU message contains special html characters. See https://expensify.slack.com/archives/C049HHMV9SM/p1680254182283689

@pecanoro @sobitneupane @tienifr Should we fix this before it's deployed to production?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it's already in production

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eh2077 I was unable to reproduce the issue on staging v1.2.93-2. Can you try reproducing it again?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try 1.2.93-4?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes. I was able to reproduce. @tienifr let's get this sorted asap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sobitneupane @pecanoro @eh2077 In

App/src/libs/actions/IOU.js

Line 556 in df10eeb

moneyRequestAction.originalMessage.comment,
we can use Str.htmlDecode(moneyRequestAction.originalMessage.comment) because moneyRequestAction.originalMessage.comment is encoded. What do you think?

</Text>
</Text>
<Icon src={Expensicons.ArrowRight} fill={props.shouldAllowViewDetails ? StyleUtils.getIconFillColor(getButtonState(props.isHovered)) : themeColors.transparent} />
Expand Down
3 changes: 1 addition & 2 deletions src/components/ReportTransaction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, {Component} from 'react';
import PropTypes from 'prop-types';
import {View} from 'react-native';
import Str from 'expensify-common/lib/str';
import styles from '../styles/styles';
import CONST from '../CONST';
import * as IOU from '../libs/actions/IOU';
Expand Down Expand Up @@ -77,7 +76,7 @@ class ReportTransaction extends Component {
wrapperStyles={[styles.reportTransactionWrapper]}
>
<Text style={[styles.chatItemMessage]}>
{Str.htmlDecode(this.props.action.message[0].html)}
{this.props.action.message[0].text}
</Text>
</ReportActionItemSingle>
{this.props.canBeRejected && (
Expand Down
5 changes: 2 additions & 3 deletions src/libs/Notification/LocalNotification/BrowserNotifications.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Web and desktop implementation only. Do not import for direct use. Use LocalNotification.
import _ from 'underscore';
import Str from 'expensify-common/lib/str';
import focusApp from './focusApp';
import * as AppUpdate from '../../actions/AppUpdate';
import EXPENSIFY_ICON_URL from '../../../../assets/images/expensify-logo-round-clearspace.png';
Expand Down Expand Up @@ -107,10 +106,10 @@ export default {
*/
pushReportCommentNotification({reportAction, onClick}, usesIcon = false) {
const {person, message} = reportAction;
const plainTextPerson = Str.htmlDecode(_.map(person, f => f.text).join());
const plainTextPerson = _.map(person, f => f.text).join();

// Specifically target the comment part of the message
const plainTextMessage = Str.htmlDecode((_.find(message, f => f.type === 'COMMENT') || {}).text);
const plainTextMessage = (_.find(message, f => f.type === 'COMMENT') || {}).text;

push({
title: plainTextPerson,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/OptionsListUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {
if (ReportUtils.isReportMessageAttachment({text: report.lastMessageText, html: report.lastMessageHtml})) {
lastMessageTextFromReport = `[${Localize.translateLocal('common.attachment')}]`;
} else {
lastMessageTextFromReport = Str.htmlDecode(report ? report.lastMessageText : '');
lastMessageTextFromReport = report ? report.lastMessageText : '';
}

const lastActorDetails = personalDetailMap[report.lastActorEmail] || null;
Expand Down
1 change: 0 additions & 1 deletion src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,6 @@ function getIOUReportActionMessage(type, total, participants, comment, currency,
const who = displayNames.length < 3
? displayNames.join(' and ')
: `${displayNames.slice(0, -1).join(', ')}, and ${_.last(displayNames)}`;

let paymentMethodMessage;
switch (paymentType) {
case CONST.IOU.PAYMENT_TYPE.EXPENSIFY:
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItemFragment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ const ReportActionItemFragment = (props) => {
numberOfLines={props.isSingleLine ? 1 : undefined}
style={[styles.chatItemMessageHeaderSender, (props.isSingleLine ? styles.pre : styles.preWrap)]}
>
{Str.htmlDecode(props.fragment.text)}
{props.fragment.text}
</Text>
</Tooltip>
);
Expand Down