WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#28820 closed defect (bug) (fixed)

Focus isn't clear when previewing an oEmbed from Add Media Panel

Reported by: philipjohn Owned by: afercia
Milestone: 4.3 Priority: high
Severity: normal Version: 3.9
Component: Media Keywords: has-patch commit
Focuses: ui, accessibility Cc:
PR Number:

Description

Summary: After pasting an oEmbed URL into the "Insert from URL" portion of the Add Media Panel, tabbing away from the URL field does not provide a clear indication of focus.

Steps to reproduce:

  1. Enter the post editor
  2. Open the Add Media Panel
  3. Go to the "Insert from URL" screen
  4. Paste an oEmbed (e.g. YouTube) URL into the URL field, and wait for the embed to appear
  5. Tab away from the URL field - the focus is unclear

Further tabbing shows the focus was on the preview after tabbing away. I've replicated this using both YouTube and WordPress.tv embeds.

Attached is a screenshot showing the view after tabbing from the URL field.

Attachments (7)

focus-on-yt-preview-unclear.png (130.7 KB) - added by philipjohn 5 years ago.
focus-on-flickr-preview-unclear.png (124.3 KB) - added by philipjohn 5 years ago.
Focus on Flickr embed
28820.patch (546 bytes) - added by afercia 5 years ago.
Screen Shot 2015-02-27 at 5.09.43 PM.png (88.2 KB) - added by wonderboymusic 5 years ago.
28820.2.patch (1009 bytes) - added by afercia 5 years ago.
Focus style for MediaElement.js embeds and controls
28820.3.patch (950 bytes) - added by afercia 5 years ago.
remove prefixed property
28820.4.patch (1.3 KB) - added by afercia 5 years ago.

Download all attachments as: .zip

Change History (34)

#1 @philipjohn
5 years ago

  • Keywords 2nd-opinion added

In hindsight maybe I should have just added this to #23560 as I've also found that focus isn't great on Flickr embeds either (screenshot to follow).

@philipjohn
5 years ago

Focus on Flickr embed

This ticket was mentioned in IRC in #wordpress-ui by philipjohn. View the logs.


5 years ago

#3 @SergeyBiryukov
5 years ago

  • Component changed from Embeds to Media
  • Focuses ui added

Related: #28267

#4 @celloexpressions
5 years ago

  • Version changed from trunk to 3.9

I'm not sure if we can do much here, as the focus enters the 3rd-party embed. For Youtube videos using the html5 player, there is an indication of focus (though it isn't great).

@afercia
5 years ago

#5 @afercia
5 years ago

  • Keywords has-patch added

Proposed patch fixes at least the linked image case (e.g. Flickr).

#6 @wonderboymusic
5 years ago

  • Keywords 2nd-opinion removed

Attached screenshot of focused image with patch applied. I'll commit that, it helps.

#7 @wonderboymusic
5 years ago

In 31585:

In the Insert From URL state of the Post frame, add the necessary CSS for focus styles for images.

Example image to insert: https://flic.kr/p/rnsm5M

See #28820.

#8 @wonderboymusic
5 years ago

  • Milestone changed from Awaiting Review to 4.2

#9 @DrewAPicture
5 years ago

  • Keywords dev-feedback added
  • Owner set to afercia
  • Status changed from new to assigned

@afercia: Would you mind taking a look at this and figuring out what's left? Seems like the experience focusing just about any media in this context isn't great.

#10 @afercia
5 years ago

@drew: when embeds are rendered with MediaElement.js maybe we could try to override the focus style though maybe MediaElement.js should really provide its own, improved, style for focus. See attached patch and screenshots. Maybe the overrides should be moved to wp-mediaelement.css.

For embeds rendered in iframes, as @celloexpressions pointed out, the focus enters the 3rd-party embed. Not sure, maybe we could try to inject some CSS in the iframe? and try to target focusable elements (if any) with :focus. Some embeds don't even have focusable controls, for example Ted.com.

When 3rd parties provide their own flash player then I'm afraid it's up to the different flash plugin versions. The ActiveX version handle focus better, given that the 3rd party player actually takes care of handling focus. The NPAPI (PepperFlash too) version has well known issues with focus (since years and years...), not sure why sorry I'm really not an expert when it comes to flash.
For example the wordpress.tv embed in IE works pretty well but with the NPAPI plugin in Firefox or PepperFlash in Chrome it's impossible to move focus to the flash player controls.

MediaElement.js container focused: standard focus style

https://cldup.com/07Fk3y9o3w.png

MediaElement.js controls focused: yellow outline

https://cldup.com/wZs5p0LllG.png

Please notice maybe the volume "hint" message needs some attention:

https://cldup.com/zugTZhSSKi.png

For reference, the tested embeds:

http://playground.html5rocks.com/samples/html5_misc/chrome_japan.ogv
http://playground.html5rocks.com/samples/html5_misc/chrome_japan.webm
http://playground.html5rocks.com/samples/html5_misc/chrome_japan.mp4
http://videos.videopress.com/w0MiG12E/snow2-its-back-h2642_dvd.mp4
http://youtu.be/VXa9tXcMhXQ
https://www.youtube.com/watch?v=272aujSjIQs
http://vimeo.com/70323400
http://wordpress.tv/2014/04/16/introducing-wordpress-3-9-smith/
http://www.ted.com/talks/dong_woo_jang_the_art_of_bow_making.html
http://www.flickr.com/photos/effixe/5424743638/
http://www.dailymotion.com/video/xy19cu_the-hangover-part-iii-official-teaser-trailer_shortfilms
https://vine.co/v/hBFxTlV36Tg
http://blip.tv/hipsterhood/hipsterhood-s2-e1-hipsters-with-hangovers-6619915
http://open.spotify.com/track/4bz7uB4edifWKJXSDxwHcs
https://soundcloud.com/itamarmsjunior/stevie-ray-vaughan-scuttle-1
https://twitter.com/nacin/status/431875643089747968
http://www.meetup.com/WordPress-Meetup-Milano/
http://www.slideshare.net/haraldf/business-quotes-for-2011

@afercia
5 years ago

Focus style for MediaElement.js embeds and controls

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


5 years ago

#12 @DrewAPicture
5 years ago

  • Priority changed from normal to high

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


5 years ago

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


5 years ago

@afercia
5 years ago

remove prefixed property

#15 @afercia
5 years ago

Refreshed patch changes "FF" in "Firefox" in a comment and removes a prefixed property, as per Slack conversation.

#16 @DrewAPicture
5 years ago

@afercia: Could you just outline for us exactly what improvement/change we're looking to see in applying the patch here? It's just not really clear what we should be testing for before and after.

#17 @afercia
5 years ago

@drew: try to embed one of these:
http://playground.html5rocks.com/samples/html5_misc/chrome_japan.ogv
http://playground.html5rocks.com/samples/html5_misc/chrome_japan.webm
http://playground.html5rocks.com/samples/html5_misc/chrome_japan.mp4

the previews will be rendered with MediaElement.js, then tab away from the input field and keep tabbing. See the focus style differences before and after.

#18 @DrewAPicture
5 years ago

  • Keywords commit added

Thanks @afercia.

The focus styles are definitely improved with 28820.3.patch. Moving for commit consideration.

#19 follow-up: @helen
5 years ago

  • Keywords dev-feedback commit removed

I'm not for committing this just yet. Questions:

  1. Should we send focus styles upstream to MEjs?
  2. Why yellow for the focus style?
  3. Do we need to account for any other embed types? The YouTube example is still unclear to me, for instance, although I don't know what exactly is focused in the one tab stop that doesn't show anything.

#20 in reply to: ↑ 19 @afercia
5 years ago

Replying to helen:

  1. Should we send focus styles upstream to MEjs?

As I noted before "maybe MediaElement.js should really provide its own, improved, style for focus".

  1. Why yellow for the focus style?

Just because it's the default in other players, e.g. flash video players, with dark backgrounds.

  1. Do we need to account for any other embed types? The YouTube example is still unclear to me, for instance, although I don't know what exactly is focused in the one tab stop that doesn't show anything.

Current patch doesn't touch embeds rendered inside iframes, so Youtube embeds won't be touched. But as you noticed, with these kind of embeds (which are real "oEmbed") the problem persists, focus isn't clear. Only Webkit will sometimes, depending on cases, apply its default blue outline.

#21 @helen
5 years ago

  • Milestone changed from 4.2 to Future Release

Let's contribute upstream to MEjs - glad we fixed the image case, but going to punt on the rest of this as I'm not completely clear on what should be done in core and what else still needs fixing.

#22 @afercia
5 years ago

@helen ok :) how do you feel about keeping just the blue box-shadow around the MEjs container? That can't be fixed upstream. See:

https://cldup.com/07Fk3y9o3w.png

#23 @afercia
5 years ago

  • Keywords commit added

Refreshed patch keeps just the focus style for the embed container. Anything else should be reported upstream, namely focus style for MediaElement.js. Please notice the patch also removes some mixed space+tab characters, this is done automatically by my editor. See screenshot below.
While it's a small thing, taking care of focus style is always an improvement. Would recommend for commit consideration.

https://cldup.com/-Ew_KpG7z9.png

#24 @afercia
5 years ago

  • Milestone changed from Future Release to 4.3

@afercia
5 years ago

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


5 years ago

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


4 years ago

#27 @wonderboymusic
4 years ago

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

In 32861:

Improve focus when previewing an oEmbed from Add Media Panel

Props afercia.
Fixes #28820.

Note: See TracTickets for help on using tickets.