Skip to content

M3X-720: Enhance ins/del wrapping on atomic tags while diffing#5

Open
ragoolkr wants to merge 3 commits into
developfrom
M3X-720-atomic-tag-enhancement
Open

M3X-720: Enhance ins/del wrapping on atomic tags while diffing#5
ragoolkr wants to merge 3 commits into
developfrom
M3X-720-atomic-tag-enhancement

Conversation

@ragoolkr

@ragoolkr ragoolkr commented Jun 29, 2026

Copy link
Copy Markdown

Ticket

This pull request introduces significant improvements to the HTML diffing logic, focusing on more robust handling of atomic tags.

ISSUE

image

In html-diff when we have the tag with data-html-diff-id we consider them atomic and wrap them with ins/del. we decide opening and close of the atomic tag based on the tag name.

Sometimes we might have a structure where

<span data-htmldiff-id="testing">
  <span class="truncate">one</span>
  <span class="truncate">two</span>
</span>

then diffing algorithm finds the data-htmldiff-id starts the wrapping process it looks for the atomic tag which is <span> then it starts looking for the closing tag </span> but in our structure since it also has a child <span> the algorithm considers child's closing </span> as the parent one and produces something like this

<ins>
 <span data-htmldiff-id="testing">
   <span class="truncate">one</span>
</ins>
   <span class="truncate">two</span>
</span> // orphan tag

FIX

So now we track how many tags with same name currently open inside the atomic token old code closed atomic tag at first nested spans cut early broken markup.we count open/close depth, closes only when depth at 0 → whole nested block stays one balanced token.

working version

Core logic improvements:

  • Improved handling of atomic tags (e.g., <iframe>, <script>, and custom tags with data-htmldiff-id), ensuring each atomic tag is wrapped independently in diffs, and supporting nested and adjacent atomic tags.

Fix counts open/close depth, closes only at depth 0 → whole nested block stays one balanced token.

Testing and CI:

  • Added and updated tests for edge cases, atomic tag handling and HTML tokenization to ensure correctness and prevent regressions.

Package metadata:

  • Updated package.json to reflect new ownership (@matrixreq/htmldiff), version bump to 1.4.0, and repository links.

These changes collectively make the diffing library more reliable for complex HTML structures and easier to maintain and test.

ragoolkr added 2 commits June 29, 2026 16:48
…age nested tags and improve test coverage for edge cases
…ic tokens close correctly when depth returns to zero.
@ragoolkr ragoolkr self-assigned this Jun 29, 2026
@ragoolkr ragoolkr requested a review from sirenevenkii June 29, 2026 15:46
Comment thread js/htmldiff.js
Comment thread js/htmldiff.js
…cy; improve test coverage for self-closing tags in html_to_tokens.spec.js.
@ragoolkr ragoolkr requested a review from sirenevenkii July 3, 2026 12:20

@sirenevenkii sirenevenkii left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good. great improvement.
please, clean up the comments and styling before merging.

Comment thread js/htmldiff.js
*/
function isOpeningTagOf(word, tag){
var tagText = word.substring(word.lastIndexOf('<'));
if (tagText.indexOf('>') !== tagText.length - 1) return false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do not use inline conditions. same for closing tag

Comment thread js/htmldiff.js
* @return {boolean} True if the word is now a complete token (including the end tag),
* false otherwise.
* @return {boolean} True if word ends with an opening (non-self-closing) tag for the given
* atomic tag name.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it's not about specifically atomic tags, isn't? same for isClosingTagOf comment

Comment thread js/htmldiff.js
Comment on lines +91 to +92
// inspect the last tag in word (from its opening '<'); to an earlier '>' in that
// slice means loose text e.g. "a > b" in a <script>, not a tag, so they bail out.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why those comment are inline?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

paraphrase it, I'm struggling to understand what you mean here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants