Migrator throws tag warning for tag in comment

[1Jesper1]

HTML tags must be properly nested and closed:

https://jsfiddle.net/yj0c9f2b/

[mgol]

I don't see any error in the link you posted. Please explain the issue in more details.

[1Jesper1]

Updated: https://jsfiddle.net/hyt40ob6/ Please click on the "change color" button to trigger the error in the console.

[1Jesper1]

@mgol It should not throw a warning for elements inside comments I think.

[mgol]

Thanks for the updated test case!

The code responsible for the warning is here:

jquery-migrate/src/jquery/manipulation.js

Line 25 in c81c190

warnIfChanged( html );

The warning is printed if the jQuery.htmlPrefilter output along the logic in jQuery 3.5 was different from the input as then there's a danger updating to jQuery 3.5+ would break the code.

As that logic itself was incomplete, the Migrate version will by definition catch a lot of valid input if it only looks like it can change after the jQuery upgrade. I guess it doesn't make sense to warn if you already use jQuery 3.5.0 or newer which changed the logic to not do any replacement (see the jQuery 3.5 upgrade guide) for context but only keep it in the UNSAFE_restoreLegacyHtmlPrefilter version of the API:

jquery-migrate/src/jquery/manipulation.js

Line 19 in c81c190

warnIfChanged( html );

Does that sound good to you, @dmethvin?

@1Jesper1 would you like to submit a PR making such a change? I can guide you through the process.

[1Jesper1]

@mgol Hello, thanks for the fast reply.
I think the warning should still exist, but it should not check code inside HTML comments.
I've migrated from jQuery 3.4.1 to 3.6 and I got much occurrences where the warning is triggered, because of invalid self closing tags, like for textarea and span.
So I use the warnings from the migrator to check and fix these occurrences.
The problem was that code inside HTML comments are also checked I think.

[mgol]

If you update to jQuery 3.5.0 or newer with jQuery Migrate without calling jQuery.UNSAFE_restoreLegacyHtmlPrefilter() then your app may be broken if you haven't fixed all the inputs to jQuery DOM manipulation methods. I'm not sure if having the warnings then helps in any way. If you are calling jQuery.UNSAFE_restoreLegacyHtmlPrefilter() then the warnings would still be printed in what I propose.

The idea about the warning is to print it if jQuery <3.5 modifies the HTML input you provide to it and it would modify the input you provided by replacing the HTML comment with a different one. It's not possible to skip the logic just for comments without violating this assumption. Parsing HTML with regexes is not possible, this was the cause of the original issues with jQuery.htmlPrefilter and why we changed it in jQuery 3.5.0. We're not going to embed a full HTML parser in jQuery Migrate to detect all such edge cases, that would be huge & impractical.

Hence, I only see two possibilities: either leave the current behavior as-is or stop printing warnings in jQuery 3.5.0+ unless jQuery.UNSAFE_restoreLegacyHtmlPrefilter() is called. I'd opt for the second and not just because of what this issue is about but because we can't avoid printing warnings for completely fine input just because it'd cause problems in older jQueries.

[1Jesper1]

@mgol For me the warnings helped to detect code like: <span /> and <textarea /> which caused our app to be broken after migrating to jQuery 3.6 from jQuery 3.4.
I did not call jQuery.UNSAFE_restoreLegacyHtmlPrefilter()
It also detects Angular v1 self closed directives, which breaks in jQuery > 3.4.1.

[mgol]

What was the reason for you to not call jQuery.UNSAFE_restoreLegacyHtmlPrefilter() since, as you mentioned, your app was broken in some places due to a misuse of self-closing tags?

[1Jesper1]

@mgol Because we are updating jQuery for the security fix.

[mgol]

Just so that I understand the use case in full: you updated for the security fix, accepting that some parts of the application may be broken until you find a Migrate warning coming from their self-closing tags and until you then fix it?

[1Jesper1]

@mgol After updating jQuery I tried to fix all code, so I used the migrate script to detect broken code I overlooked. I don't accept the application to be broken, I'm trying to fix all occurences which broke due to the update. That's why I saw the warning and checked the possible broken code, but it was a false positive because of the self closing tag problem being inside the html comment.