WordPress.org

Make WordPress Core

Opened 6 weeks ago

Last modified 6 weeks ago

#48948 new defect (bug)

Fix the Site Health template to produce valid HTML

Reported by: afercia Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.2
Component: Site Health Keywords: has-screenshots needs-patch
Focuses: Cc:
PR Number:

Description (last modified by afercia)

Working on an external project that adds tests to Site Health, I noticed the JavaScript issue template produces invalid HTML, depending also on the passed content.

See the template, specifically the parts related to the issue description and actions:
https://core.trac.wordpress.org/browser/trunk/src/wp-admin/site-health.php?rev=46586&marks=151-156#L143

<script id="tmpl-health-check-issue" type="text/template">
	...
	<div id="health-check-accordion-block-{{ data.test }}" class="health-check-accordion-panel" hidden="hidden">
		{{{ data.description }}}
		<div class="actions">
			<p class="button-container">{{{ data.actions }}}</p>
		</div>
	</div>
</script>

Description

It's not wrapped within other HTML elements other than the main <div> thus there are no nesting problems and any HTML can be used.

However, some WordPress tests pass content that's actually an unordered list within a paragraph. See for example https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-site-health.php?rev=46797&marks=1640-1649#L1638

Browsers are tolerant and their parsers will try to produce well-formed HTML thus actually rendering empty paragraphs:

http://cldup.com/w5oC9shd5I.png

This is responsibility of some specific WordPress tests that output invalid HTML and should be fixed in the tests.

Actions

The template wraps the actions within a paragraph. This assumes that the content passed for the actions needs to be content that can be nested within a paragraph, for example a simple link. However, most of the tests pass content that is already wrapped in a paragraph thus producing nested paragraphs.

Also, I can't seem to be able to find any documentation on the recommended HTML that should be passed in order do have valid HTML. To help plugin authors to produce valid HTML, it would be best to:

  • either document what the recommended markup is
  • or just replace the template paragraph with a <div> thus allowing any HTML

The second option would make plugin authors responsible for the output markup: core should encourage authors to produce valid semantic HTML with good examples

Going through the WordPress tests, the content passed for the actions is inconsistent:

1
In a couple cases, the passed string is a link: it will be correctly rendered within the paragraph rendered by the template. See for example https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-site-health.php?rev=46797&marks=239-243#L238

2
More often, the actions HTML is already wrapped within a paragraph. It will be rendered within the paragraph from the template (nested paragraphs). Also in this case, the browsers parsers will try to "fix" the HTML thus producing empty paragraphs.

Also, worth noting the action is actually rendered within a paragraph that doesn't have the button-container class so this CSS class can't be used for styling:

http://cldup.com/4Dy8ncZKeS.png

3
Sometimes, the action is an empty string because there's no action to take or because the test passed: an empty paragraph will be rendered. In these cases, it would be better to not render the entire actions <div>. A simple check for data.actions may help.

4
Seems to me the get_test_php_default_timezone test is the only one that doesn't set actions in the result array, not sure why. Within the template data.actions will be undefined. I guess it should pass an empty string for consistency.

Related (in a way): #47210

Change History (2)

#1 @afercia
6 weeks ago

There are also cases where a test may want to pass multiple actions, as core itself does:

http://cldup.com/y2OYCMwvsj.png

I'd tend to think plugin authors should be allowed to make a decision on the HTML they want to use. For example, they may want to use a paragraph for each action or maybe an unordered list to group the actions or any other HTML. Instead, the core template wraps the actions within a paragraph by default, which seems to me an assumption and limitation that should be removed.

#2 @afercia
6 weeks ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.