WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#27243 closed defect (bug) (fixed)

Improve video rendering

Reported by: wonderboymusic Owned by: wonderboymusic
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.9
Component: Media Keywords:
Focuses: Cc:
PR Number:

Description

Things to look at:

  • m4vs should load natively in Chrome
  • videos should display in the same aspect ratio in the following scenarios: Edit Media screen, video shortcode, playlist shortcode, attachment page
  • videos should display on attachment pages in the default themes <-- they do not
  • videos should uniformly adapt to $content_width

Attachments (5)

video-fixes.diff (4.8 KB) - added by wonderboymusic 6 years ago.
27243.diff (1.1 KB) - added by wonderboymusic 6 years ago.
13-and-12.diff (1.2 KB) - added by wonderboymusic 6 years ago.
attachment-pages.diff (5.2 KB) - added by wonderboymusic 6 years ago.
27243.2.diff (3.6 KB) - added by wonderboymusic 6 years ago.

Download all attachments as: .zip

Change History (19)

#1 @nacin
6 years ago

Adapting to $content_width can be problematic, at least if not done in a dynamic on-the-fly fashion. See also #21256 and others.

#2 @wonderboymusic
6 years ago

In 27328:

  • Videos should always render at the same aspect ratio.
  • Don't force a pseudo-mime-type for .m4v files
  • Uniformly adapt to $content_width when setting video dimensions on the front end
  • Add the height attribute to the initial <video> in the video playlist JS template
  • Add some defensive/responsive CSS for a/v on the Edit Media page

See #27243.

@wonderboymusic
6 years ago

This ticket was mentioned in IRC in #wordpress-dev by wonderboymusic. View the logs.


6 years ago

#4 @wonderboymusic
6 years ago

In 27330:

in TwentyFourteen: don't add the has-post-thumbnail class or print the featured image when viewing an attachment.

See #27243.

#5 @wonderboymusic
6 years ago

In 27331:

In TwentyTwelve and TwentyThirteen, don't print the featurd image when viewing an attachment.

See #27243.

#6 @wonderboymusic
6 years ago

  • Keywords commit added

27243.2.diff updates the patch.

I think this is a crucial part of this release. Here is an overview of what it accomplishes for attachment pages:

2010
Without patch: Displays a sad link
With patch: Displays a sad link

2011
Without patch: Displays a sad link
With patch: Displays a RESPONSIVE video, adds 2 CSS rules

2012
Without patch: Displays a sad link
With patch: Displays a RESPONSIVE video, adds 2 CSS rules

2013
Without patch: Displays a WEIRD sad link
With patch: Displays a RESPONSIVE video, adds 3 (whoa now) CSS rules

2014
Without patch: Displays a WEIRD sad link, styling is broken
With patch: Displays a RESPONSIVE video, NO CSS REQUIRED

The only other change is to prepend_attachment() - instead of displaying the sad link, it displays (wait for it...) the attachment. 2014 needs to be patched to fix the broken page. Let's treat ourselves right and go all the way. Lance was already amenable to this last week. I didn't land it over the weekend because I was working on launching a side project.

#7 @SergeyBiryukov
6 years ago

Re: [27330] and [27331]: I might be missing something, but why would has_post_thumbnail() return true for attachments? I could not reproduce that.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#8 @wonderboymusic
6 years ago

Since 3.6, if you declare support for post-thumbnails for attachment:audio or attachment:video, when you upload a file that contains image bits in its ID3 tags, the image is uploaded for you and associated as the featured image.

Playlists support images for each item.

#9 @SergeyBiryukov
6 years ago

Do we need to include the same check in [27429] then?

#10 @wonderboymusic
6 years ago

I would say yes.

This ticket was mentioned in IRC in #wordpress-dev by wonderboymusic. View the logs.


6 years ago

#12 @wonderboymusic
6 years ago

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

In 27622:

On attachment pages for audio and video, display players. Currently, only a link is displayed. Add minimal CSS rules for compatibility with 2011, 2012, and 2013 themes.

In prepend_attachment, add logic to support attachment types that are not image.

In get_post_class(), don't add the has-post-thumbnail class for attachment pages.

Fixes #27243.

#13 @kovshenin
6 years ago

  • Keywords has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

In r27622, can we please not use the alternative if/elseif/else syntax? The only good reason for the alternative syntax is when control structures are injected in HTML, like the loop in theme template files.

#14 @wonderboymusic
6 years ago

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

In 27682:

Do not use the alternative if/elseif/else syntax in prepend_attachment() introduced in [27622], as per kovshenin.

Fixes #27243.

Note: See TracTickets for help on using tickets.