WordPress.org

Make WordPress Core

#23955 closed defect (bug) (fixed)

Twenty Thirteen: Embedded media positioned incorrectly at <999px breakpoints

Reported by: sabreuse Owned by: markjaquith
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: needs-testing has-patch
Focuses: Cc:

Description (last modified by sabreuse)

At tablet and smaller sizes, embedded media are shifted offscreen to the left rather than being centered in the layout. Patch attached.

props to alun for reporting the issue and the fix here: http://wordpress.org/support/topic/twenty-thirteen-responsive-layout-for-entry-media?replies=1

Attachments (12)

sidemedia.diff (452 bytes) - added by sabreuse 15 months ago.
2013-video-truncated.png (81.8 KB) - added by sabreuse 15 months ago.
2013-video-patched.png (206.2 KB) - added by sabreuse 15 months ago.
23955.diff (1.6 KB) - added by obenland 15 months ago.
23955.2.diff (3.0 KB) - added by obenland 15 months ago.
23955.3.diff (469 bytes) - added by obenland 15 months ago.
Re-introduce max-widths for audio post formats. There was a reason we had them.
23955.video-width.diff (1.9 KB) - added by obenland 15 months ago.
23955.video-width.2.diff (1.3 KB) - added by obenland 15 months ago.
23955.4.diff (503 bytes) - added by obenland 15 months ago.
Make sure to only alter the width on the frontend.
23955.5.diff (1.8 KB) - added by wonderboymusic 15 months ago.
23955.6.diff (2.5 KB) - added by wonderboymusic 15 months ago.
23955.7.diff (2.5 KB) - added by wonderboymusic 15 months ago.

Download all attachments as: .zip

Change History (30)

sabreuse15 months ago

comment:1 sabreuse15 months ago

  • Description modified (diff)

comment:2 SergeyBiryukov15 months ago

  • Component changed from General to Bundled Theme
  • Milestone changed from Awaiting Review to 3.6
  • Version set to trunk

comment:3 follow-up: lancewillett15 months ago

  • Keywords has-patch reporter-feedback added

@sabreuse Could you help us out by attaching before and after screenshots? Also, browser info is always helpful for repeating the bugs.

comment:4 in reply to: ↑ 3 sabreuse15 months ago

  • Keywords reporter-feedback removed

Replying to lancewillett:

@sabreuse Could you help us out by attaching before and after screenshots? Also, browser info is always helpful for repeating the bugs.

Sure thing -- sorry I didn't think of it before:

Currently, seeing this on both Chrome & Firefox:


After patch:


obenland15 months ago

comment:5 obenland15 months ago

  • Keywords needs-testing added

Sabreuse's patch went in the right direction, but we need a bit more verbose. I only tested the patch with images, have not had the chance to test it with videos.

We need to account for all screen sizes, with and w/o a sidebar, LTR and RTL.

obenland15 months ago

comment:6 lancewillett15 months ago

In 23964:

Twenty Thirteen: better handling of media in main content area, accounting for a sidebar and all viewport width possibilities. Props sabreuse and obenland, see #23955.

comment:7 lancewillett15 months ago

  • Keywords has-patch removed

Leaving this ticket open for more testing.

obenland15 months ago

Re-introduce max-widths for audio post formats. There was a reason we had them.

comment:8 lancewillett15 months ago

Seems somewhere along the line video embeds, like YouTube URLs stopped being filtered to the correct width of 724, with twentythirteen_content_width() that fires on 'template_redirect'. It's broken on the index view or any view where the video post isn't the first in the results. (Works OK on single.)

comment:9 lancewillett15 months ago

In 23971:

Twenty Thirteen: reinstate Audio post format max-width layout handling, lost in r23964. See #23955, props obenland.

comment:10 lancewillett15 months ago

In 23972:

Twenty Thirteen: adjust content_width value for video shortcodes in video post formats and on attachment templates. See #23955, props obenland.

obenland15 months ago

Make sure to only alter the width on the frontend.

comment:11 obenland15 months ago

.4 restricts the new width to the frontend.

wonderboymusic15 months ago

comment:12 wonderboymusic15 months ago

  • Keywords has-patch added

if you have a giant video and then filter only width, and it happens to equal content width, the aspect ratio is going to get effed up - I think constraints need to happen in the shortcode, which every video passes through. My patch only works if the 2013 filter isn't there. Let's discuss.

My patch is necessary anyways to constrain giant videos from rendering bigger than $content_width in the admin and on the front end

wonderboymusic15 months ago

comment:13 wonderboymusic15 months ago

23955.6.diff​ fixes TwentyThirteen as well

wonderboymusic15 months ago

comment:14 wonderboymusic15 months ago

23955.7.diff​ uses $content_width instead of hard-coding 724

comment:15 wonderboymusic15 months ago

use .6.diff instead of .7.diff - $content_width is dynamic in some cases

comment:16 markjaquith15 months ago

Needs refresh.

What exactly is the problem at this juncture? How can I test it to verify that the patch fixes it?

comment:17 markjaquith15 months ago

Nevermind, doesn't need refresh.

comment:18 markjaquith15 months ago

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

In 23989:

Constrain large videos from rendering bigger than $content_width on both frontend and backend.

props wonderboymusic. fixes #23955.

Note: See TracTickets for help on using tickets.