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 These regression cases come under the two categories of regressions, other two regressions are related to performance so those are separate. |
Uzaifm127
changed the title
1 participant
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:
<code>example.comexpected:
<code><a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a>Case:
<pre>example.comexpected:
<pre><a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a>Case:
<a>example.comexpected:
<a><a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a>Case:
<video>example.comexpected:
<video><a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a>Case:
example.comfollowed by a new line and`</a>`expected:
<a href="https://example.com" target="_blank" rel="noreferrer noopener">example.com</a><br /><code></a></code>Case:
<code>before.com<pre>inside.com</pre>after.comexpected:
<code><a href="https://before.com" target="_blank" rel="noreferrer noopener">before.com</a><pre>inside.com</pre><a href="https://after.com" target="_blank" rel="noreferrer noopener">after.com</a>Case:
<a>*bold* ~strike~ <unfinished after.comexpected:
<a><strong>bold</strong> <del>strike</del> <unfinished <a href="https://after.com" target="_blank" rel="noreferrer noopener">after.com</a>Case:
<unfinished example.com \code``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: Complete and valid HTML parsing
Repeat steps 1-5 from Test 1.
Replace the invitation message with:
Invite the member and inspect the
welcomeNotepayload.Verify that the
spanremains unchanged and thatexample.comis not incorrectly converted into a link inside it.Repeat the same test with the following valid or nested HTML cases:
<h1>example.com</h1><a><span>*bold*</span></a><code><h1>example.com</h1></code><code title="a<b">example.com</code><code><code>example.com</code>after.com</code> outside.comVerify the following behavior:
example.cominside<h1>is linked as in the existing parser.*bold*inside the nested anchor/span structure is formatted correctly.<inside the quoted attribute does not break HTML parsing.outside.comis linked.Verify that no unexpected error appears in the JS console.
Test 3: 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 4: 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari