Conversation
Keep surrounding raw HTML boundaries when validating URL and Markdown candidates so unfinished and protected tags retain the previous parser behavior.
Uzaifm127
changed the title
Preserve raw HTML context across nested, malformed, and multi-marker URL and Markdown inputs. Bound dot-heavy hostname candidates to avoid expensive URL-regex work
The raw HTML check in canUseCandidateScanning function already handles this case, so remove the extra marker tracking
These are the malformed/incomplete HTML cases which we covered for first category of regressions:
expected:
expected:
expected:
expected:
expected:
expected:
expected:
expected:
expected:
expected:
expected:
expected:
expected: expected:
expected:
expected:
expected:
expected: These are the valid/complete HTML cases we covered for second category of regressions
expected:
expected:
expected:
expected:
expected:
expected: Note
|
Uzaifm127
changed the title
Add Given, When, and Then comments explaining the setup, expected behavior, and purpose of the long-input and raw HTML tests.
Explain the hostname checks, TLD limit, HTML boundary filtering, and incomplete-tag handling in the optimized parser.
|
I haven't tested the Test 1 in iOS safari and Android native because there was no payload in Android native and I have some issues with iOS safari web inspector but I don't think it should be problematic as I attached the terminal based testing in macOS section. TY! |
Uzaifm127
changed the title
|
Maybe better for thiss to be reviewed by @JS00001 ? |
|
@carlosmiceli Sorry for the review request, you are automatically requested for review. This PR will be reviewed by @linhvovan29546 and @JS00001 TY. |
|
Yep, we will review, I need them to comment here so I can assign them |
3 participants
Explanation of Change
This is a follow up PR for ExpensiMark performance optimization for #95210.
The original optimization avoids running the autolink, bold, and strikethrough regexes against the complete message on every edit. Instead, it finds small possible Markdown and URL candidates first, then validates only those candidates with the existing regexes.
While comparing the optimized parser with the previous parser, we found four regression categories:
shouldEscapeText: false, such as unfinished tags, unclosed protected tags, unmatched closing tags, and Markdown followed by>. These cases could produce different URL, bold, or strikethrough output than the old full-text regex.shouldEscapeText: false, such as<span>example.com</span>,<h1>example.com</h1>,<a><span>*bold*</span></a>, nested code/pre tags, and<inside quoted HTML attributes..comxcandidates. For example,aaaa...aaaa.comxis not a valid URL, but the scanner could previously treat.comas a possible TLD and send a very large invalid candidate to the URL regex.a.a.a.a... upto 9k characters. The text is not a URL, but the scanner could create a large candidate and the URL regex spent time rejecting it.To preserve compatibility, the optimized candidate scanner now runs only when it is safe. When
shouldEscapeTextisfalseand the text contains raw<or>characters, ExpensiMark uses the original full-text regex behavior instead. This preserves the old parser behavior for both malformed and valid raw HTML.The URL scanner now also validates hostname labels and requires a known TLD with a valid boundary before creating a URL candidate. This prevents long
.comxand dot-heavy plain-text inputs from reaching the expensive URL regex.Fixed Issues
$ #95210
PROPOSAL:
Tests
Test 1: Malformed HTML and HTML-boundary parsing
Open the App on Web.
Open the Devtool by right clicking on the App and select a inspect or press
F12.In Chrome DevTools, select the Network tab.
Go to Workspaces > workspace > Members > Invite member.
Enter an email address that is not already a workspace member.
Continue to the invitation message step.
Replace the invitation message with:
Clear the Network panel in devtool.
Click Invite.
Open the
AddMembersToWorkspaceAPI request.Inspect the
welcomeNotein the payload of API.Verify that both
before.comandafter.comare converted into links.Repeat the same test with the following cases:
Case:
expected:
<a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a><br /><code></a></code>Case:
expected:
<unfinished <a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a> <code>code</code>Case:
<unfinished example.com 😄expected:
<unfinished <a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a> <emoji>😄</emoji>Case:
<unfinished example.com [label](https://example.com)expected:
<unfinished <a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a> <a href="https://example.com" target="_blank" rel="noreferrer noopener">label</a>Case:
<unfinished example.com expected:
<unfinished <a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a> <img src="https://example.com/image.png" alt="alt" />Case:
<unfinished example.com expected:
<unfinished <a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a> <video data-expensify-source="https://example.com/video.mp4" >video</video>Case:
# heading <unfinished example.comexpected:
<h1>heading <unfinished <a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a></h1>Case:
<unfinished example.com @hereexpected:
<unfinished <a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a> <mention-here>@here</mention-here>Case:
*bold* >expected:
*bold* >Case:
~strike~ >expected:
~strike~ >Case:
*one* > *two*expected:
<strong>one* > *two</strong>Case:
~one~ > ~two~expected:
<del>one~ > ~two</del>Test 2: Long invalid
.comxURL candidateaaaaaaaaaaa...... upto 8000 characters then .comx, for example:aaaaaaaa....8000.comxNote
Follow the following steps to copy the correct text to test:
1. Open Chrome DevTools and select the Console tab.
2. Run:
copy('a'.repeat(8496) + '.comx')Type several characters quickly at the end of the text.
Verify that:
.comxis not converted into a link.Repeat the test with:
copy('a'.repeat(8496) + '.com')but make sure to add a space after.comat the end of the text.Verify that the valid
.comtext becomes a link and that typing remains responsive after adding a space at the end.Test 3: Dot-heavy invalid URL candidate
a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.... up to 8000 characters.Note
Follow the following steps to copy the correct text to test:
1. Open Chrome DevTools and select the Console tab.
2. Run:
copy('a.'.repeat(4499) + 'a')Paste the text into the composer.
Type several characters quickly at the end.
Verify that:
Offline tests
Same as Test
QA Steps
Same as Test
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Test 2
Android-native-2.mov
Test 3
Android-native-3.mov
Android: mWeb Chrome
Test 1
Android-mweb-1.mp4
Test 2
Android-mweb-2.mov
Test 3
Android-mweb-3.mov
iOS: Native
Test 1
iOS-native-1.mp4
Test 2
iOS-native-2.mp4
Test 3
iOS-native-3.mp4
iOS: mWeb Safari
Test 2
iOS-mweb-2.mov
Test 3
iOS-mweb-3.mov
MacOS: Chrome / Safari
Test 1
macOS-1.mov
Test 2
macOS-2.mov
Test 3
macOS-3.mov
Terminal based test
terminal-based-test.mp4