Make WordPress Core

Opened 10 years ago

Closed 8 years ago

Last modified 8 years ago

#38759 closed defect (bug) (fixed)

Accessibility glitch when editing an embedded video

Reported by: Presskopp Owned by: afercia
Priority: normal Milestone: 5.1
Component: Media Version: 3.9
Severity: normal Keywords: has-screenshots has-patch
Cc: Focuses: accessibility

Description

to reproduce: make a new page and enter video shortcode for embedding a video like

[video src="https://www.youtube.com/watch?v=KF8bGYks-bk"]

in text mode. Switch to Visual Mode, click to edit the video.

Above the URL of the video will be an area (titled SRC) where you have a mouse pointer like if it was a link, but it isn't. See screenshot..

The label "SRC" is hardcoded in \wp-includes\media-template.php L.1044/1127

Attachments (3)

video-hover.jpg​ (156.5 KB ) - added by Presskopp 10 years ago.
38759.diff​ (5.9 KB ) - added by afercia 9 years ago.
38759.2.diff​ (6.4 KB ) - added by afercia 9 years ago.

Download all attachments as: .zip

Change History (21)

#1 @joemcgill
10 years ago

  • Component General → Media
  • Focuses accessibility added

Thanks @Presskopp is this new to Trunk or is this reproducible in the current stable version of WP (4.6.1) as well?

#2 @joemcgill
10 years ago

  • Owner set to joemcgill
  • Status new → reviewing

#3 @Presskopp
10 years ago

same in 4.6.1

#4 @joemcgill
10 years ago

  • Keywords needs-patch added
  • Milestone Awaiting Review → Future Release
  • Version trunk

Thanks for confirming. Since this isn't a regression, let's look at this in a future release.

#5 @lukecavanagh
10 years ago

@Presskopp

Confirmed the issue is reproducible in 4.7-beta3-39201.

#6 @afercia
10 years ago

  • Keywords has-screenshots added
  • Version → 3.9

SRC is just the label for the input field. In other places, for example the attachments browser, a similar field has a URL label and the field is readonly but not disabled. This allows, at least, to copy and select the URL:

https://cldup.com/dbW4JTiM3o.png

I'd suggest to rename the label and make the field (also the other ones for additional sources) enabled and readonly.

An additional a11y issue here is that, inside the label, there's also the button to remove the video source. This button should be moved out from the label.

Side note: I think this was added in [27411] and the buttons later.

Last edited 10 years ago by afercia (previous) (diff)

@afercia
9 years ago

#8 @afercia
9 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone Future Release → 4.9

Looking a bit into this, there are a few occurrences in the media templates where <label> elements are used incorrectly:

  • to wrap just text (no form controls)
  • to wrap an input field and a button
  • to wrap a mix of the two cases above

Label elements should be used just for labelable elements. 38759.diff​ is a first attempt to fix most of these cases and produce valid, semantic markup. As per the ​accessibility standards, labels should be associated using for/id attributes.

The patch can be certainly improved and need testing. Given how so badly associated labels impact accessibility, I'd like to propose to try to fix this in 4.9.

Last edited 9 years ago by afercia (previous) (diff)

#9 @Presskopp
9 years ago

  • Keywords needs-testing added

This ticket was mentioned in ​Slack in #accessibility by afercia. ​View the logs.


9 years ago

This ticket was mentioned in ​Slack in #core by jeffpaul. ​View the logs.


9 years ago

#12 @jbpaul17
9 years ago

  • Milestone 4.9 → 4.9.1

Per discussion in today's 4.9 bug scrub, we're punting this to 4.9.1.

#13 @johnbillion
9 years ago

  • Milestone 4.9.1 → 5.0

@afercia
9 years ago

#14 @afercia
9 years ago

  • Keywords needs-testing removed
  • Owner changed from joemcgill to afercia

Refreshed patch, includes also a few minor coding standards. Since we're at the beginning of the 5.0 release cycle, I'd propose to merge this now, as this is the best way to test it extensively.

#15 @afercia
9 years ago

  • Resolution → fixed
  • Status reviewing → closed

In 42444:

Accessibility: Media: Improve the usage of a few label elements in the media templates.

Label elements should only be used for labelable elements.

  • Uploaded By and Uploaded To aren't form controls and shouldn't be associated with labels
  • changes the labels for media source, alternate sources, poster image, and tracks to solve a layout issue and explicitly associate the labels to their form fields (previously, the labels were wrapping also the Remove buttons)

Props Presskopp, afercia.
Fixes #38759, #40468.

#16 @SergeyBiryukov
8 years ago

  • Keywords fixed-major added
  • Milestone 5.0 → 4.9.9
  • Resolution fixed
  • Status closed → reopened

Reopening for 4.9.9 consideration, per the latest Accessibility team bug scrub meeting.

#17 @pento
8 years ago

  • Keywords fixed-major removed
  • Milestone 4.9.9 → 5.1
  • Resolution → fixed
  • Status reopened → closed

#18 @pento
8 years ago

In 43829:

Accessibility: Media: Improve the usage of a few label elements in the media templates.

Label elements should only be used for labelable elements.

  • Uploaded By and Uploaded To aren't form controls and shouldn't be associated with labels
  • changes the labels for media source, alternate sources, poster image, and tracks to solve a layout issue and explicitly associate the labels to their form fields (previously, the labels were wrapping also the Remove buttons)

Merges [42444] to the 5.0 branch.

Props Presskopp, afercia.
Fixes #38759, #40468.

Note: See TracTickets for help on using tickets.