Update list of self-closing tags

[christianwach]

I keep getting HTML tags must be properly nested and closed warnings because the <source> tag is not declared in the rxhtmlTag pattern. Seems as though it could be usefully updated to include a fuller set of self-closing tags, e.g.

var rxhtmlTag = /<(?!area|base|br|col|embed|hr|img|input|link|meta|param|source|track|wbr)(([a-z][^\/\0>\x20\t\r\n\f]*)[^>]*)\/>/gi,

Thanks!

[mgol]

Thanks for the report.

The regex is what's included in jQuery 3.4.1, see https://github.com/jquery/jquery/blob/3.4.1/src/manipulation.js#L39. The purpose is to mimic that version behavior so that any HTML string that would be replaced by jQuery.htmlPrefilter in jQuery 3.4.1 would also be replaced here, triggering the warning.

That said, for void elements like <source>, <source></source> is equivalent to <source> in HTML so maybe we could add them to the list as well. Here's the specced list:
https://html.spec.whatwg.org/multipage/syntax.html#void-elements

There's always some risk of apps breaking because this replacement was doing an incorrect thing the app now depends on and if we stop replacing the tag, it will break. I suspect the risk is very low here, though.

@jquery/core what do you think? The problem is if code tries to be both HTML & XHTML-compliant then they have to use self-closing tags for void elements which triggers Migrate warnings if we don't add them all to the list.

@christianwach would you be interested in a PR for the regex?

[christianwach]

@mgol Thanks for your response. I wanted to open an issue first to see if there were reasons for the current list. I'm happy to open a PR, but it looks like it'd be a good idea to keep this repo in sync with jQuery too. I'll pause for the time being to see how others respond.

[mgol]

Keeping it in sync with jQuery means not changing it since that regex was removed from jQuery 3.5.0; that's the reason why it's in Migrate. I think updating it should be fine but yeah, let's wait for what others thing before spending time on a fix.

[christianwach]

Ha, good point! 🤦

[mgol]

@christianwach Looking at it again, the logic in Migrate is not just comparing input with output but it's also seeing if the resulting HTML has any differences. Therefore, if those self-closing tags behave in the same way whether the input is replaced by jQuery or not, there would be no warning. Since you've said you're seeing warnings, this suggests you have a real issue to fix.

Can you modify your Migrate copy locally so that just before the line where warnings are set you're also logging the output of makeMarkup( html ) and makeMarkup( changed )? This would show what has changed and why Migrate is reporting it.

[christianwach]

@mgol Sure thing. Consider this snippet:

<video width="320" height="240" controls>
  <source src="movie.mp4" type="video/mp4" />
  <source src="movie.ogg" type="video/ogg" />
  Your browser does not support the video tag.
</video>

The current implementation transforms this to:

<video width="320" height="240" controls>
  <source src="movie.mp4" type="video/mp4"></source>
  <source src="movie.ogg" type="video/ogg"></source>
  Your browser does not support the video tag.
</video>

Since <source> is not declared as self-closing, this (of course) triggers the HTML tags must be properly nested and closed warning.

[mgol]

Thanks, but it's not exactly what I asked about. It's not about the HTML string before & after jQuery transforms it as this in itself is not enough to trigger the warning. Migrate, when the before & after strings differ, constructs inert DOM subtrees setting innerHTML of the before & after contents into two elements and compares their resulting innerHTML after they're transformed by the browser. Here it seems to me it should be identical. And, indeed, you can see in this JS Bin there's no Migrate warning: https://jsbin.com/duxatebare/1/edit?html,js,console,output

Can you share the outputs of relevant makeMarkup calls that do trigger a warning?

[mgol]

@christianwach hey, could you answer my last question?

[christianwach]

@mgol Ah sorry, been snowed under with work lately.

You seem to be right about the transforms... my mistake. I thought the <source> element was the relevant element in the markup because the code above was the literally only change when I diffed the before and after code - and altering the regex as per my OP makes the issue go away.

So, given that the markup is a full page generated by WordPress, it seems likely that other factors are at play here too. I can point you to examples of the issue in the wild but perhaps it's a bit of a whack-a-mole situation given the variability of the context.

I'll close this issue for now since I'm happy to work with a modified copy of jQuery Migrate for development purposes - and other can find it if they want to revive it.

Thanks for your feedback and attention Michał.