Html5 / W3C Validation

[arena]

I am using Firefox + HTML Validator

This add-on is detecting (using w3c online validation) some html 5 errors.

Mostly on lists (posts, users, search field) etc ...

I think this requires a complete review of wp admin to stick to the w3c standards.

Thank you

[SergeyBiryukov]

Replying to arena:

This add-on is detecting (using w3c online validation) some html 5 errors.

Could you give some examples?

[arena]

​https://validator.w3.org/ for posts list

[pento]

Confirmed 3 bugs on the posts list page:

• <input type="number" step="1" min="1" max="999" class="screen-per-page" name="wp_screen_options[value]" id="edit_post_per_page" maxlength="3" value="20" />

maxlength="3" is invalid for <input type="number" />. This is a hangover from when it used to be type="text"; max="999" does what we want it to do. The maxlength attribute can be removed.

• The "View" link for each post has rel="permalink". This is from the WCLR spec, which is obsolete. permalink should be replaced with bookmark, from the ​HTML5 spec.

• In the Quick Edit table, there's a colspan="<?php echo $this->get_column_count(); ?>", but it doesn't seem to be necessary. It's a bit of a classic, introduced in [8973], when the Quick Edit table became an entire table, instead of just another row in the post list table. It can go.

[ocean90]

The maxlength attribute can be removed.

It's still used by browsers which don't support input[type="number"] and fall back to a normal text input.

[pento]

Replying to ocean90:

It's still used by browsers which don't support input[type="number"] and fall back to a normal text input.

IE 9 and lower (~1% of browsers), which will fall back to the server-side validation.

Given that it doesn't affect the server, and only has a very minor effect on a UX edge case, I lean towards removing it.

[mihai2u]

I confirm this bug is still present, so I'm taking a stab to patch it as my first contributing ticket.

[mihai2u]

screenshot-media-page-no-validator-error

[mihai2u]

posts-page-no-validator-errors

[mihai2u]

The patch.

[mihai2u]

I've removed the colspan and the class="colspanchange" on that <td> element, since it was no longer necessary without a colspan being present. I confirmed that everything looks good, just like in the attached screenshot even without that class. Actually there are no css styles attached to that class, and it's just common.js changing the count on interactions in a multi-column table. Since these tables only had one single column, it had to be dropped to avoid any unnecessary overhead.

I replaced rel="permalink" by rel="bookmark" in 4 locations, as they all gave validation errors (expected).

The maxlength invalid attribute has been dropped just as well.

[topher1kenobe]

Removed colspan tag, changed rel="permalink" to rel="bookmark" and removed maxlength="3".

Not as comprehensive as @mihai2u's I didn't remove the colspanchange class.

[michalzuber]

Looks good, but class="colspanchange" should stay; wp-admin/js/common.js is using it.

[mihai2u]

I'm refreshing this, and thanks michalzuber for your contribution of finding the usage of colspanchange inside that .js file.

I've pushed this on Git on top of the latest WP and the fixes still work in achieving HTML validation:
​https://github.com/xwp/wordpress-develop/pull/221
( and the github diff: ​https://github.com/xwp/wordpress-develop/pull/221.diff )

This matches the developments inside the 37004.3.diff from above, and only refreshed. I'll attach the 37004.4.diff matching the git PR as well.

[mihai2u]

Received feedback on the GitHub PR from Miina that the Comments list, Users list, Pages list have validations errors.
I'm going to update this in order to cover validation for those pages as well.

[jbpaul17]

Updated keywords to match current status. Will need to see who can review this and potentially schedule for a planned release once the patch is ready.

[mihai2u]

I've done further progress on this task and achieved w3c valid pages in a couple more areas where errors were spotted:
Comments index - removed template colspan updated by JS according to the screen size
Users index - fixed duplicate IDs being present on two buttons
Settings - General - removed inline width on th element, as it's obsolete (width was already defined in CSS)
Settings - General - fixed incorrect positioning of closing span tag around the Custom date format options
Settings - Media - fixed incorrect usage of colspan="2" in the case where there were no other rows/columns in the same table.

I looked into some other w3c errors, and they were related to two things:
1 - mark-up used as a JS template which had table mark-up although it was a single row (posts / tags, posts / categories)
2 - duplicate IDs on the Pages index (the dropdown builder automatically populates the id with the name value passed as a parameter, and there isn't an easy way to make it generate dropdowns with no IDs).

I've updated the github branch with the latest stable WP version, and will upload the corresponding 37004.5.diff for it ( ​https://github.com/xwp/wordpress-develop/pull/221.diff )

[jbpaul17]

Adding Needs Testing keyword to ensure this applies cleanly before looking to milestone the patch.

[jbpaul17]

Milestoning to 4.8 to help get code review on this over next month or so.

[stubgo]

Tested that the following pages are fixed and without validation errors:

• Comments
• Media
• Posts
• Plugins
• Users

[mihai2u]

I've merged in the latest Wordpress master and all applies well without any conflicts:
​https://github.com/xwp/wordpress-develop/pull/221
(i'll attach the updated .diff - no changes to the original work)

With this refreshed once more, can it go forward with being merged in @swissspidy ?

[afercia]

See #40610 for the maxlength attribute.

Also, I think the colspan attribute must stay, since it is used by colSpanChange(), unless I'm missing something.

[mihai2u]

@afercia

The colspan attributes I removed were always dynamically populated by JS. The colSpanChange() function looks at elements with a "colspanchange" class from what I saw in the code and adds the colspan attribute and value automatically.
The JS does not require the attribute to be present (with an empty or placeholder value), because the attribute gets created with the JS call to set the attribute to a particular value. I verified that the areas where the placeholder colspan was removed from HTML still function well with javascript populating the correct values according to which page / browser size the element appears on.

Regarding your reference to #40610 - this offers the same patch in the same fashion as that ticket there - the removal of the same attribute.
If you'd like me to remove it from this ticket and allow the patch to come through #40610 instead, just ask and I'll accommodate. Browser-based input attribute validation has always been sketchy at best.

Looking forward to getting a checkmark on this ticket, standing by to further support it to completion.

[afercia]

The JS does not require the attribute to be present

Actually the colSpanChange() function accepts a parameter which is the number to add to the colspan attribute, if present. It does check for the presence of a colspan attribute in the markup:

colSpanChange : function(diff) {
	var $t = $('table').find('.colspanchange'), n;
	if ( !$t.length )
		return;
	n = parseInt( $t.attr('colspan'), 10 ) + diff; <-- check for the table colspan attribute and adds the passed param
	$t.attr('colspan', n.toString()); <-- finally, updates the colspan attribute with the new value
}

[mihai2u]

Hmm. I'll have to look closer. I haven't looked at the source code specifically, but at how the page looked before and after the colspan removal (and I saw the colspan was added by JS), but didn't really check what number was inside.

Thank you very much for quoting the JS for it.

[mihai2u]

@afercia

I've refreshed the patch and added back the colspan attributes even if they break validation.
I pondered with the idea of using a data-colspan-diff="" attribute and use that in the JavaScript when present, but I discarded it as I don't think it brings much value. If you find value in going that approach to achieve validation on those particular areas (it's 3 templates receiving the HTML error), I can act on that.

This is the error received: "Table columns in range 2…5 established by element td have no cells beginning in them." -> and that happens because the colspan is part of a template that's used by JS in order to be placed in the proper table which has the columns in that range. Statically speaking, if there's no JS making use of them, they indeed appear as inside a table with not enough columns to make the colspan valid.

I hope all of the remaining code improvements can find their way onto 4.8.1

Thank you for your review.

[afercia]

@mihai2u thanks for refreshing the patch! To clarify, the colspan attribute is a valid attribute in HTML5, see the spec: ​https://www.w3.org/TR/html5/tabular-data.html#the-td-element

The quick edit and bulk edit forms work in a very special way. The forms are inside a table that is initially hidden and it's placed at the bottom of the page. This table works as sort of "placeholder" for the quick/bulk edit forms and it's made of two rows with just one column cell (<td>) with a colspan attribute. When activated, the table row containing the form fields gets injected at the top of the list table (bulk edit) or after the table row of a single post (quick edit).

In the list tables, some columns can be optionally displayed or hidden: users can choose which columns to display from the Screen Options panel at the top of the page. That means there's no way to predict how many columns a table has.

So, when the quick/bulk edit table row gets injected and become visible, it needs to know how many columns are currently set to be displayed in the table and update its colspan attribute accordingly, otherwise the layout will break.
Note that users can also hide/display columns while the quick/bulk edit form is open, and when this happens, the JS needs to update on the fly the colspan attribute value.

The validator message is actually different depending on the table you're validating and the initial value of the colspan attribute. Checking the post list table, for me is:
Table columns in range 2…7 established by element td have no cells beginning in them.

That doesn't mean the colspan attribute is not allowed. It just means the validator "sees" a table cell with td colspan="7" so that column is supposed to stretch across seven columns but that table doesn't contain other rows with 7 columns. It's the hidden table at the bottom of the page that works as "placeholder" for the quick/bulk edit rows.

It's a bit complicated to explain. In a few words, I wouldn't be so worried because the "error" the validator sees is in a hidden table used just for printing out a template. It's not available to assistive technologies and doesn't break anything.

[mihai2u]

Great, thank you for the thorough explanation @afercia
Then I believe we should be moving forward with the other fixes I've made to improve the HTML validity and that would be it for this ticket.

Good first bug? :)

Looking forward to the commit to core and putting this to sleep.

[afercia]

Replying to pento:

The "View" link for each post has rel="permalink". This is from the WCLR spec, which is obsolete. permalink should be replaced with bookmark

For history: introduced in [480] on 10/26/03 :)

[afercia]

Refreshed patch. I'd say to take care of maxlength in #40610.

Spotted a few more things, most notably: $detach_url in the media list table needs to be escaped. Other things:

• 1 more permalink rel attribute in Multisite
• missing span in wp-admin/includes/template.php (in the "Attach media" modal)
• let's make that table row in the Users screen use a td instead of a th, there's no need for a table header there

[mihai2u]

Thank you for the update, afercia.
I'll do another refresh which would handle the items you referenced.

[afercia]

@mihai2u the latest patch should already cover those items.

[mihai2u]

Thanks for the clarification, afercia. I thought you were asking me to make those updates.
Then it's should be smooth sailing towards getting committed.

[afercia]

Turns out I was wrong about $detach_url, the validation error there is about the square brackets, not the ampersand. Also, there's a long standing discussion about the whole concept of "Attach" and "Detach" (see #6820) and there's a chance they will be removed in the future. I'm not comfortable in trying to fix that link now that we're in beta 2, so I'm leaving it out.

[afercia]

In 40823:

Administration: Fix some HTML validation errors.

Fixes some minor HTML issues in the admin and, most notably, changes the rel
attribute used in the List Tables from rel="permalink" to rel="bookmark".

Props mihai2u, pento, arena, topher1kenobe, michalzuber, stubgo.
Fixes #37004.