WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#23831 closed defect (bug) (fixed)

Insert Into Post for Audio / Video doesn't always work

Reported by: wonderboymusic Owned by:
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Post Formats Keywords: has-patch commit
Focuses: Cc:

Description

As Otto pointed out, "Link To" can be set to "None" which was sending a broken shortcode to the editor. The HTML sent to the editor should be based on what is selected using "Link To" in the Media modal.

Attachments (8)

23831.diff (2.6 KB) - added by wonderboymusic 7 years ago.
23831.2.diff (5.0 KB) - added by wonderboymusic 7 years ago.
23831.3.diff (5.0 KB) - added by wonderboymusic 7 years ago.
23831.4.diff (5.0 KB) - added by wonderboymusic 7 years ago.
23831.5.diff (655 bytes) - added by wonderboymusic 7 years ago.
flashmediaelement.swf (28.0 KB) - added by wonderboymusic 7 years ago.
23831.6.diff (71.6 KB) - added by wonderboymusic 7 years ago.
23831.7.diff (390 bytes) - added by kovshenin 7 years ago.

Download all attachments as: .zip

Change History (34)

@wonderboymusic
7 years ago

#1 @wonderboymusic
7 years ago

the attached patch makes the shortcode always work, but doesn't change when the Link To option changes. Working on that next.

#2 @lancewillett
7 years ago

  • Keywords needs-refresh added

Is this still a bug?

#3 @wonderboymusic
7 years ago

Will in-westi-gate

#4 @wonderboymusic
7 years ago

  • Keywords commit added; needs-refresh removed

this is still necessary - the patch still applies cleanly and needs to be committed, but the ticket shouldn't be closed yet, need to decide what to do when someone selects "Link to Attachment Page" or whatever

#5 @wonderboymusic
7 years ago

this is mainly for the content editor, not the format meta

#6 @markjaquith
7 years ago

In 23870:

Fix some Insert-into-Post for Audio/Video bugs.

props wonderboymusic. see #23831.

#7 follow-up: @wonderboymusic
7 years ago

attachment:23831.2.diff​ enforces dimensions for video, when present, in all cases. The post format meta for video is now a shortcode, so the dimensions don't have to be looked up on the fly. For audio and video, the meta can be an attachment id, url, HTML embed code, or a shortcode

Introduces attachment_url_to_postid() which will try to convert a URL containing a home_url() front into post ID by looking at the guid column for an attachment in the posts table

The issue Otto brought up, which is the original reason for this ticket, still exists. The question is: should the content change for Insert Into Post for audio or video when the user selects the custom, file, post or none URL option. As it stands, all insert the proper shortcode, there is no difference based on that selection

#8 in reply to: ↑ 7 ; follow-up: @azaozz
7 years ago

Replying to wonderboymusic:

Introduces attachment_url_to_postid() which will try to convert a URL containing a home_url() front into post ID by looking at the guid column for an attachment in the posts table

This will not work on many sites. The guid is _not_ the url to an attachment when the site or the uploads directory has been moved. To do something like this you will need to look at _wp_attached_file post meta, etc. Check out wp_get_attachment_url().

#9 @wonderboymusic
7 years ago

Refreshed against latest trunk

#10 @SergeyBiryukov
7 years ago

Wouldn't url_to_postid() work for attachments? From my testing, it does.

#11 @wonderboymusic
7 years ago

last I checked, only checks attachment page, not the actual file

#12 @wonderboymusic
7 years ago

I am going to redo that func as per Ozz

#13 @SergeyBiryukov
7 years ago

Replying to wonderboymusic:

last I checked, only checks attachment page, not the actual file

Indeed, I only tested with attachment pages.

#14 @markjaquith
7 years ago

In 23969:

Enforce video dimensions.

props wonderboymusic. see #23831.

#15 @wonderboymusic
7 years ago

23831.5.diff fixes the 404 - dynamic rendering of file types that trigger Flash or Silverlight was the issue

#16 @markjaquith
7 years ago

23831.5.diff doesn't fix the 404 for me. The 404 happens on the front end and the back end. This fixes neither for me.

#17 @wonderboymusic
7 years ago

Can you confirm your cache is empty? I was able to reproduce the error repeatedly until I cleared Chrome browser cache

#18 @markjaquith
7 years ago

My cache is completely empty. Multiple browsers. Still can reproduce the issue.

Did you upload the wrong patch or forget to add a file to the stage? Looking at .5.diff, I can't imagine why you'd think that would have any effect on the front of the site.

#20 @wonderboymusic
7 years ago

It still works for me locally, not sure what is up - I pasted the URL on GitHub

#21 @wonderboymusic
7 years ago

23831.6.diff + flashmediaelement.swf​ are latest for MEjs 2.11.3 - works for me... could use other testers. Need to select videos that trigger the Flash or Silverlight plugin so that we can confirm the .swf and .xap paths aren't 404ing

#22 @markjaquith
7 years ago

In 23977:

Update MediaElement.js to 2.11.3

props wonderboymusic, John Dyer. see #23831.

@kovshenin
7 years ago

#23 @kovshenin
7 years ago

Missed a semicolon in [23870], see 23831.7.diff.

#24 @SergeyBiryukov
7 years ago

In 23998:

Add missing semicolon. props kovshenin. see #23831.

#25 in reply to: ↑ 8 @SergeyBiryukov
7 years ago

Replying to azaozz:

Replying to wonderboymusic:

Introduces attachment_url_to_postid() which will try to convert a URL containing a home_url() front into post ID by looking at the guid column for an attachment in the posts table

This will not work on many sites. The guid is _not_ the url to an attachment when the site or the uploads directory has been moved. To do something like this you will need to look at _wp_attached_file post meta, etc. Check out wp_get_attachment_url().

Sounds like a valid concern to me.

#26 @wonderboymusic
7 years ago

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

#24458 covers the above

Note: See TracTickets for help on using tickets.