WordPress.org

Making WordPress.org

Opened 3 months ago

Last modified 10 days ago

#4730 reopened defect

unbalanced HTML in Plugins Home page

Reported by: joyously Owned by:
Milestone: Priority: normal
Component: Plugin Directory Keywords: needs-patch good-first-bug
Cc:

Description

If you run https://wordpress.org/plugins/ through a validator, you get quite a mess.
The main things are:

  • stray </span> tag for each plugin shown
  • duplicate ID post-xxxxx and plugin-slug for each (?)
  • <style> not allowed as child of element <a> for each plugin
  • unclosed <div> (for div with id="page")

Both https://validator.w3.org/nu/ and https://html5.validator.nu/ gave those errors, in addition to lots of other warnings.

Attachments (1)

4730.diff (970 bytes) - added by dufresnesteven 11 days ago.
Diff to add "Alt" tags to icon images.

Download all attachments as: .zip

Change History (22)

#1 @joyously
3 months ago

  • stray </span> tag for each plugin is in the theme

https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/themes/pub/wporg-plugins/template-parts/plugin.php#L60

  • duplicate ID post-xxxxx and plugin-slug

I think this is because some plugins can show in multiple sections since they are order_by rand.

  • <style> not allowed as child of element <a>

The style is output here:
https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/themes/pub/wporg-plugins/client/components/plugin-icon/index.jsx#L37
Perhaps it should be srcset instead?

  • unclosed <div> (for div with id="page") is in the theme

It should be here:
https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/themes/pub/wporg-plugins/footer.php#L17

#2 @dd32
3 months ago

AFAIK we can't use srcset as most of these need to be explicitly background images, which is why it's CSS in the first place.

We can probably drop the ID's though and switch to classes, so it won't matter if a plugin appears multiple times or has multiple <style>'s output.

This ticket was mentioned in Slack in #meta by joyously. View the logs.


3 months ago

#4 @dd32
8 weeks ago

  • Keywords needs-patch good-first-bug added

This ticket was mentioned in Slack in #meta by joyously. View the logs.


8 weeks ago

#6 @dd32
7 weeks ago

In 9230:

Plugin Directory: Fix some HTML validation errors.

  • Remove duplicate ID's
  • Don't nest <style> under <a>

See #4730.

#7 @dd32
7 weeks ago

In 9231:

Plugin Directory: Close the #page div.

Props joyously.
See #4730.

#8 @dd32
7 weeks ago

In 9232:

Plugin Directory: Remove a stray </span>.

Props joyously.
See #4730.

#9 @dd32
7 weeks ago

In 9233:

Plugin Directory: revert splitting <style> out of the icon, keep the id -> class change.

Partially Reverts [9230].
See #4730.

#10 @dd32
7 weeks ago

In 9234:

Plugin Directory: revert splitting <style> out of the icon, keep the id -> class change.

Missed from r9233.
Partially Reverts [9230].
See #4730.

#11 @dd32
7 weeks ago

  • Resolution set to fixed
  • Status changed from new to closed

Fixed the issues that are fixable.

<style> not allowed as child of element <a> for each plugin is going to stay, because the only way to "fix" it is to remove the <style> from the body entirely as it's "improper" to be there and should only ever be in the <head> - But realistically, browsers don't care. Apparently the ability to use <style> within <body> was removed from the HTML5 draft. :shrug:

#12 @dd32
7 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Actually, you're right, apologies @joyously - We probably can switch these plugin icons to an <img> element with srcset..

It's the not-proper-sized images which we have trouble with, and the banner header on singular pages. The banner image is a background image for reasons I can't recall now, but using an <img> element there wasn't going to work, I'm not sure what it was.

We're not going to remove the inline <style> tags everywhere, but slimming this particular piece of HTML down might be worth it.

#13 @dd32
7 weeks ago

In 9235:

Plugin Directory: Trial using <img> for plugin icons.

Props joyously.
See #4730.

#14 @dd32
7 weeks ago

In 9238:

Plugin Directory: Plugin Icons: Use the 2x srcset format instead.

See #4730.

@dufresnesteven
11 days ago

Diff to add "Alt" tags to icon images.

#15 @dufresnesteven
11 days ago

@dd32

Diff: attachment:4730.diff

I added some alt tags to the plugin icons.

#16 follow-up: @dd32
11 days ago

Thanks @dufresnesteven.

I'm actually not even sure that this should have an alt, and if it should, if it shouldn't just be an alt="".
The image is almost purely for decoration, and the plugin name will always be included next to/after it in terms of a screen reader, so I'm actually thinking that this will increase the burden upon the viewer without actually providing any benefit of context. Thoughts?

The %s plugin logo

As an FYI, I think this should only be referred to as the plugin icon since it's not always the logo of the plugin, and may also be auto-generated.

#17 @joyously
10 days ago

Actually, it is recommended to use alt="" for decorative images. Here's one reference: https://www.w3.org/WAI/tutorials/images/decorative/

#18 in reply to: ↑ 16 ; follow-up: @dufresnesteven
10 days ago

There is definitely debate as to whether an icon that does have text next to it needs an alt tag and based on the component itself, I would agree it may not be necessary. But if we look at this component in the flow, it isn't apparent.

For example, on the /plugins page, tabbing through doesn't go through the expected flow. Instead of reading the Heading to contextualize the section, we go directly to the "See All" link and then to the plugin itself.

It sounds like this when you tab into the Block Enabled Plugins section:

"Main section heading: See all block enabled plugins, link"

Tab to select a plugin

"Article"

Tab to icon

"Icon XXXXXX, link"

Tab to title

It then reads the name of the plugin.

If the plugin doesn't have the word "Plugin" in it, theres no way to tell you are focused on a plugin.

You can try this extension to validate:
https://chrome.google.com/webstore/detail/chromevox-classic-extensi/kgejglhpjiefppelpmljglcjbhoiplfn

Note: we should probably also add a title to the <a> tag that surrounds the <img> since there is no way to tell where it's going.

Replying to dd32:

Thanks @dufresnesteven.

I'm actually not even sure that this should have an alt, and if it should, if it shouldn't just be an alt="".
The image is almost purely for decoration, and the plugin name will always be included next to/after it in terms of a screen reader, so I'm actually thinking that this will increase the burden upon the viewer without actually providing any benefit of context. Thoughts?

The %s plugin logo

As an FYI, I think this should only be referred to as the plugin icon since it's not always the logo of the plugin, and may also be auto-generated.

#20 in reply to: ↑ 18 ; follow-up: @dd32
10 days ago

Replying to dufresnesteven:

It sounds like this when you tab into the Block Enabled Plugins section:

"Main section heading: See all block enabled plugins, link"

Tab to select a plugin

"Article"

Tab to icon

"Icon XXXXXX, link"

Tab to title

It then reads the name of the plugin.

To me, that sounds like we either need to hide the <a> for the icon from screen readers (There's probably an aria-* tag for that?) or move the <img> into the plugin links <a>?

#21 in reply to: ↑ 20 @dufresnesteven
10 days ago

Yep, I think that would definitely be the best approach for the plugin items but it still doesn't address that fact that the pages that surround these plugin items don't have proper tabbing and context. Unless we are willing to go back and address the pages that surround these items, I believe more information is really the best solution. With that being said, I have plenty to learn when it comes to accessibility, do we have any experts around?

Replying to dd32:

To me, that sounds like we either need to hide the <a> for the icon from screen readers (There's probably an aria-* tag for that?) or move the <img> into the plugin links <a>?

Note: See TracTickets for help on using tickets.