Skip to content

Commit da0bcab

Browse files
abhinavkrinabhinavkrin
authored
fix: no highlighting of non-latin highlights (#35124)
1 parent 893f4e4 commit da0bcab

File tree

5 files changed

+271
-5
lines changed

5 files changed

+271
-5
lines changed

‎.changeset/purple-laws-joke.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@rocket.chat/meteor': patch
3+
---
4+
5+
Fixes an issue where non-Latin highlights were inconsistently applied, ensuring non-Latin characters are reliably detected and highlighted.

‎apps/meteor/app/lib/server/functions/notifications/messageContainsHighlight.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,18 @@ export function messageContainsHighlight(message: Pick<IMessage, 'msg'>, highlig
1414
return false;
1515
}
1616

17+
const leftBoundary = '(?<=^|[^\\p{L}\\p{N}_])';
18+
const rightBoundary = '(?=$|[^\\p{L}\\p{N}_])';
19+
1720
return highlights.some((highlight: string) => {
18-
const hl = escapeRegExp(highlight);
19-
const regexp = new RegExp(`(?<!:)\\b${hl}\\b:|:\\b${hl}(?!:)\\b|\\b(?<!:)${hl}(?!:)\\b`, 'i');
21+
// Due to unnecessary escaping in escapeRegExp, we need to remove the escape character for the following characters: - = ! :
22+
// This is necessary because it was crashing the client due to Invalid regular expression error.
23+
const hl = escapeRegExp(highlight).replace(/\\([-=!:])/g, '$1');
24+
const pattern =
25+
`(?<!:)${leftBoundary}${hl}${rightBoundary}:` +
26+
`|:${leftBoundary}${hl}${rightBoundary}(?!:)` +
27+
`|${leftBoundary}(?<!:)${hl}(?!:)${rightBoundary}`;
28+
const regexp = new RegExp(pattern, 'iu');
2029
return regexp.test(message.msg);
2130
});
2231
}

‎apps/meteor/client/components/GazzodownText.spec.tsx‎

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
import { Markup } from '@rocket.chat/gazzodown';
2+
import { mockAppRoot } from '@rocket.chat/mock-providers';
3+
import { render, screen } from '@testing-library/react';
4+
5+
import GazzodownText from './GazzodownText';
6+
import { useMessageListHighlights } from './message/list/MessageListContext';
7+
8+
jest.mock('@rocket.chat/ui-client', () => ({
9+
useFeaturePreview: () => false,
10+
}));
11+
jest.mock('@rocket.chat/fuselage-hooks', () => ({
12+
useLocalStorage: () => ['en', jest.fn()],
13+
}));
14+
jest.mock('./message/list/MessageListContext', () => ({
15+
useMessageListHighlights: jest.fn(),
16+
}));
17+
jest.mock('../lib/utils/fireGlobalEvent', () => ({
18+
fireGlobalEvent: jest.fn(),
19+
}));
20+
jest.mock('../views/room/hooks/useGoToRoom', () => ({
21+
useGoToRoom: jest.fn(),
22+
}));
23+
24+
const mockUseMessageListHighlights = useMessageListHighlights as jest.MockedFunction<typeof useMessageListHighlights>;
25+
26+
const wrapper = mockAppRoot().withUserPreference('useEmojis', true).withSetting('UI_Use_Real_Name', false).withJohnDoe();
27+
28+
const HighlightTester = ({ text }: { text: string }) => {
29+
return (
30+
<Markup
31+
tokens={[
32+
{
33+
type: 'PARAGRAPH',
34+
value: [{ type: 'PLAIN_TEXT', value: text }],
35+
},
36+
]}
37+
/>
38+
);
39+
};
40+
41+
describe('GazzodownText highlights', () => {
42+
beforeEach(() => {
43+
jest.clearAllMocks();
44+
});
45+
46+
it('should highlight Russian word "тест" in the middle of a sentence', () => {
47+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'тест' }] as any);
48+
49+
render(
50+
<GazzodownText>
51+
<HighlightTester text='Это тест сообщения' />
52+
</GazzodownText>,
53+
{ legacyRoot: true, wrapper: wrapper.build() },
54+
);
55+
// Expect that the highlighted element wraps exactly "тест"
56+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^тест$/i);
57+
});
58+
59+
it('should highlight Russian word "тест" at the beginning of a sentence', () => {
60+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'тест' }] as any);
61+
62+
render(
63+
<GazzodownText>
64+
<HighlightTester text='Тест сообщения' />
65+
</GazzodownText>,
66+
{ legacyRoot: true, wrapper: wrapper.build() },
67+
);
68+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^тест$/i);
69+
});
70+
71+
it('should not highlight "тест" when it is part of a larger word', () => {
72+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'тест' }] as any);
73+
74+
render(
75+
<GazzodownText>
76+
<HighlightTester text='Тестирование сообщения' />
77+
</GazzodownText>,
78+
{ legacyRoot: true, wrapper: wrapper.build() },
79+
);
80+
expect(screen.queryByTitle('Highlighted_chosen_word')).not.toBeInTheDocument();
81+
});
82+
83+
it('should highlight Russian word "тест" at the end of a sentence', () => {
84+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'тест' }] as any);
85+
86+
render(
87+
<GazzodownText>
88+
<HighlightTester text='Сообщение тест' />
89+
</GazzodownText>,
90+
{ legacyRoot: true, wrapper: wrapper.build() },
91+
);
92+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^тест$/i);
93+
});
94+
95+
it('should highlight English word "test" regardless of its position', () => {
96+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'test' }] as any);
97+
98+
render(
99+
<GazzodownText>
100+
<HighlightTester text='This is a test message' />
101+
</GazzodownText>,
102+
{ legacyRoot: true, wrapper: wrapper.build() },
103+
);
104+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^test$/i);
105+
});
106+
107+
it('should highlight all occurrences of the highlighted word in different positions', () => {
108+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'test' }] as any);
109+
110+
render(
111+
<GazzodownText>
112+
<HighlightTester text='test is at the beginning, in the middle test, and at the end test' />
113+
</GazzodownText>,
114+
{ legacyRoot: true, wrapper: wrapper.build() },
115+
);
116+
const highlightedElements = screen.getAllByTitle('Highlighted_chosen_word');
117+
// Expect three separate highlights.
118+
expect(highlightedElements.length).toBe(3);
119+
highlightedElements.forEach((el) => {
120+
expect(el).toHaveTextContent(/^test$/i);
121+
});
122+
});
123+
124+
it('should highlight the highlighted word in a multiline text', () => {
125+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'test' }] as any);
126+
127+
const multilineText = `First line
128+
Test line
129+
Another line with test
130+
in it.`;
131+
132+
render(
133+
<GazzodownText>
134+
<HighlightTester text={multilineText} />
135+
</GazzodownText>,
136+
{ legacyRoot: true, wrapper: wrapper.build() },
137+
);
138+
const highlightedElements = screen.getAllByTitle('Highlighted_chosen_word');
139+
// At least two occurrences are expected: one for "Test" (capitalized) and one for "test"
140+
expect(highlightedElements.length).toBeGreaterThanOrEqual(2);
141+
// Verify that the highlighted texts match "test" (case-insensitive)
142+
highlightedElements.forEach((el) => {
143+
expect(el.textContent).toMatch(/^test$/i);
144+
});
145+
});
146+
147+
it('should highlight highlighted word when surrounded by colons', () => {
148+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'test' }] as any);
149+
150+
render(
151+
<GazzodownText>
152+
<HighlightTester text='This is :test: inside colons' />
153+
</GazzodownText>,
154+
{ legacyRoot: true, wrapper: wrapper.build() },
155+
);
156+
// The highlighted element should contain only "test"
157+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^test$/i);
158+
});
159+
160+
it('should highlight highlighted word when colon is at the start', () => {
161+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'test' }] as any);
162+
163+
render(
164+
<GazzodownText>
165+
<HighlightTester text='This is :test with colon at the start' />
166+
</GazzodownText>,
167+
{ legacyRoot: true, wrapper: wrapper.build() },
168+
);
169+
// The highlighted element should contain only "test"
170+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^test$/i);
171+
});
172+
173+
it('should highlight highlighted word when colon is at the end', () => {
174+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'test' }] as any);
175+
176+
render(
177+
<GazzodownText>
178+
<HighlightTester text='This is test: with colon at end' />
179+
</GazzodownText>,
180+
{ legacyRoot: true, wrapper: wrapper.build() },
181+
);
182+
// The highlighted element should contain only "test"
183+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^test$/i);
184+
});
185+
186+
it('should highlight multiple different highlighted words in the same text', () => {
187+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'test' }, { highlight: 'highlight' }] as any);
188+
189+
render(
190+
<GazzodownText>
191+
<HighlightTester text='This test message should highlight the word highlight in multiple places: test and highlight.' />
192+
</GazzodownText>,
193+
{ legacyRoot: true, wrapper: wrapper.build() },
194+
);
195+
196+
const highlightedElements = screen.getAllByTitle('Highlighted_chosen_word');
197+
198+
expect(highlightedElements.length).toBe(5);
199+
const texts = highlightedElements.map((el) => el.textContent?.toLowerCase());
200+
expect(texts).toEqual(expect.arrayContaining(['test', 'highlight']));
201+
});
202+
203+
it('should highlight a word containing special characters like "-", "_", ".", "/", "=", "!", ":', () => {
204+
// The highlight word includes special characters.
205+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'te-st_te.st/te=te!st:' }] as any);
206+
207+
const testText = 'This message contains te-st_te.st/te=te!st: as a highlighted word.';
208+
render(
209+
<GazzodownText>
210+
<HighlightTester text={testText} />
211+
</GazzodownText>,
212+
{ legacyRoot: true, wrapper: wrapper.build() },
213+
);
214+
215+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^te-st_te\.st\/te=te!st:$/i);
216+
});
217+
218+
it('should highlight the word in a case-insensitive manner', () => {
219+
mockUseMessageListHighlights.mockReturnValue([{ highlight: 'Test' }] as any);
220+
221+
render(
222+
<GazzodownText>
223+
<HighlightTester text='This is a test message' />
224+
</GazzodownText>,
225+
{ legacyRoot: true, wrapper: wrapper.build() },
226+
);
227+
expect(screen.getByTitle('Highlighted_chosen_word')).toHaveTextContent(/^test$/i);
228+
});
229+
});

‎apps/meteor/client/components/GazzodownText.tsx‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,12 @@ const GazzodownText = ({ mentions, channels, searchText, children }: GazzodownTe
3838
return;
3939
}
4040

41-
const alternatives = highlights.map(({ highlight }) => escapeRegExp(highlight)).join('|');
42-
const expression = `(?=^|\\b|[\\s\\n\\r\\t.,،'\\\"\\+!?:-])(${alternatives})(?=$|\\b|[\\s\\n\\r\\t.,،'\\\"\\+!?:-])`;
41+
// Due to unnecessary escaping in escapeRegExp, we need to remove the escape character for the following characters: - = ! :
42+
// This is necessary because it was crashing the client due to Invalid regular expression error.
43+
const alternatives = highlights.map(({ highlight }) => escapeRegExp(highlight).replace(/\\([-=!:])/g, '$1')).join('|');
44+
const expression = `(?<=^|[\\p{P}\\p{Z}])(${alternatives})(?=$|[\\p{P}\\p{Z}])`;
4345

44-
return (): RegExp => new RegExp(expression, 'gmi');
46+
return (): RegExp => new RegExp(expression, 'gmiu');
4547
}, [highlights]);
4648

4749
const markRegex = useMemo(() => {

‎apps/meteor/tests/unit/app/lib/server/functions/notifications/messageContainsHighlight.tests.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,25 @@ describe('messageContainsHighlight', () => {
6666
};
6767
expect(messageContainsHighlight(message, ['thumbsup'])).to.be.true;
6868
});
69+
70+
it('should return true when find a cyrillic highlight in the end of the message', async () => {
71+
const message = {
72+
msg: 'highlighted regular Привет',
73+
};
74+
expect(messageContainsHighlight(message, ['Привет'])).to.be.true;
75+
});
76+
77+
it('should return true when find a cyrillic highlight in the beginning of the message', async () => {
78+
const message = {
79+
msg: 'Привет highlighted regular',
80+
};
81+
expect(messageContainsHighlight(message, ['Привет'])).to.be.true;
82+
});
83+
84+
it('should return true when find a cyrillic highlight in the middle of the message', async () => {
85+
const message = {
86+
msg: 'highlighted Привет regular',
87+
};
88+
expect(messageContainsHighlight(message, ['Привет'])).to.be.true;
89+
});
6990
});

0 commit comments

Comments
 (0)