WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 9 months ago

#24449 closed defect (bug) (fixed)

Attachment display settings in the Media Library are ignored for audio and video inside 'Standard' post format and pages

Reported by: sennza Owned by: nacin
Milestone: 3.6 Priority: high
Severity: blocker Version: 3.6
Component: Media Keywords: has-patch i18n-change
Focuses: Cc:

Description

Sometimes both clients and I will upload audio and video files through the media uploader and link them to Attachment Pages so that people can do the old right click -> Save link as... thang.

Currently in trunk the Link to Attachment Page in the "ATTACHMENT DISPLAY SETTINGS" is ignored for audio and video and the new [audio] and [video] shortcodes are applied no matter what is selected.

I can understand that this is desirable behaviour in both the Audio and Video post formats to reduce confusion for the user but I think it would be acceptable to remove those shortcodes in the Standard post format and for Pages (and CPTs) as well. i.e. Give the user some control when acceptable.

To reproduce this upload either an .mp3 or .mp4 through the media uploader in a Standard Post format or in a Page and under "ATTACHMENT DISPLAY SETTINGS" choose "Link To Attachment Page" and the shortcodes for audio and video will be apply so that MediaElement.js will be triggered.

I'm not sure if this is the desired behaviour so I'll tag this as a 2nd-opinion.

Attachments (13)

standard-post-link-to-attachment-page.png (864.5 KB) - added by sennza 11 months ago.
Standard Post Link To Attachment Page For Audio
audio-shortcode.png (145.3 KB) - added by sennza 11 months ago.
Audio Shortcode Is Generated
standard-post-link-to-attachment-page-video.png (856.9 KB) - added by sennza 11 months ago.
Standard Post Link To Attachment Page For Video
video-shortcode.png (144.2 KB) - added by sennza 11 months ago.
Video Shortcode Is Generated
wp_ajax_send_attachment_to_editor.diff (1.1 KB) - added by derekspringer 10 months ago.
Patch for wp_ajax_send_attachment_to_editor() function to allow inserting links to media instead of shortcode.
wp_ajax_send_attachment_to_editor.v2.diff (1.2 KB) - added by derekspringer 10 months ago.
Updated version of the patch which uses the title of the file when linking to the media if it's set, otherwise falls back to the basename of the file.
24449.diff (3.7 KB) - added by nacin 9 months ago.
24449.2.diff (6.9 KB) - added by nacin 9 months ago.
24449.3.diff (7.0 KB) - added by nacin 9 months ago.
24449.4.diff (8.2 KB) - added by nacin 9 months ago.
24449.5.diff (710 bytes) - added by nacin 9 months ago.
24449.6.diff (1.5 KB) - added by nacin 9 months ago.
24449.7.diff (4.2 KB) - added by nacin 9 months ago.

Download all attachments as: .zip

Change History (29)

sennza11 months ago

Standard Post Link To Attachment Page For Audio

sennza11 months ago

Audio Shortcode Is Generated

sennza11 months ago

Standard Post Link To Attachment Page For Video

sennza11 months ago

Video Shortcode Is Generated

comment:1 sennza11 months ago

  • Cc bronson@… added

comment:2 SergeyBiryukov11 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:3 kovshenin10 months ago

  • Cc kovshenin added

comment:4 designsimply10 months ago

Tested and confirmed. No matter what option is selected for "Linked To" when inserting an .mp3 file into a post, the audio shortcode is entered.

My steps to reproduce:

  1. Go to Posts -> Add New
  2. Click the Add Media button
  3. Upload a new .mp3 file or select one you have previously uploaded
  4. Find the "ATTACHMENT DISPLAY SETTINGS" heading on the right
  5. Select "Custom URL" for the "Linked To" option
  6. Click the "Insert into post" button
  7. Repeat steps 2 through 6 for Media File, Attachment Page, and None
  • Expected: a link to appear instead of an audio shortcode for Custom URL and Attachment Page, actually I'm not sure what should happen if you select "None"—I'm not even sure which option is supposed to insert the shortcode (aka media player) versus the other options (?)
  • Actual: an audio shortcode is entered no matter what you select for "Linked To" when inserting an .mp3 file

http://f.cl.ly/items/0Q1g1v1x3C3o0Y332g0Z/Screen%20Shot%202013-06-18%20at%2011.02.36%20AM.png

http://f.cl.ly/items/3P1L2V2k2G0O27151a2W/Screen%20Shot%202013-06-18%20at%2011.02.59%20AM.png

Tested with WP trunk 24433 on Bluehost with Chrome 27.0.1453.110 on Mac OS X 10.8.3. I was using a Standard post format and the Twenty Thirteen theme during this test. I didn't try any other post formats.

comment:5 helen10 months ago

#24609 was marked as a duplicate.

derekspringer10 months ago

Patch for wp_ajax_send_attachment_to_editor() function to allow inserting links to media instead of shortcode.

comment:6 derekspringer10 months ago

Unknowingly created a duplicate report in #24609. Here's the patch I created to fix the problem:
http://core.trac.wordpress.org/attachment/ticket/24449/wp_ajax_send_attachment_to_editor.diff

Essentially I added a check to see if the attachment display settings passed anything (as it did in previous versions) and used that instead of the new media shortcodes. Additionally, the links that get passed now include the link itself as the anchor text, otherwise the editor will chuck the link since it has no text.

On a related note: the 'Custom URL' option doesn't make a ton of sense for audio or video as it stands. But I think that's a separate ticket :)

Last edited 10 months ago by derekspringer (previous) (diff)

comment:7 SergeyBiryukov10 months ago

  • Keywords has-patch added; 2nd-opinion removed

derekspringer10 months ago

Updated version of the patch which uses the title of the file when linking to the media if it's set, otherwise falls back to the basename of the file.

comment:8 derekspringer10 months ago

I spent some more time thinking about linking directly to the media and worked out a way to use the file's title as the anchor text if it's set, otherwise using the basename of the file. I think this makes a bit more sense for users and they'll have to spend less time manually editing the links that get inserted.

comment:9 nacin9 months ago

The most basic thing to do here is to simply hide the attachment display settings for audio and video attachments.

The slightly more complicated thing is to make it so they can choose between embedding and linking. "Attachment Page" and "Media File" make sense as a "Link To", while "Custom URL" and "None" do not. Moreover, it's not "Link to", it's actually what to insert.

It's really a whole other setting entirely, with these options:

  • Embed this audio/video
  • Link to this media file
  • Link to attachment page

For 3.6, we obviously are doing embedding by default. But that's quite a change from people who have been used to URLs, and in some cases, they might prefer a URL. So we probably do need to do something here beyond simply removing the attachment display settings for audio/video.

comment:10 nacin9 months ago

  • Component changed from Post Formats to Media

nacin9 months ago

nacin9 months ago

comment:11 nacin9 months ago

24449.2.diff has tested pretty well for me. I tested with audio, video, PDF, and image files; and with an audio file that had an unsupported extension. (Well, I commented out a supported extension to test.)

It is missing a single piece of abstraction that is really bugging me. The logic for canEmbed is duplicated in both media-views and media-editor. It could also benefit from review from koopersmith but that can happen post-commit.

While the existing 'link' setting/prop could probably have been used, a separate 'embed' setting/prop was used to avoid possible issues with the associated user settings. Given 'data-user-setting', this may have been mostly without merit, but getUserSetting() does appear in media-editor.js. I wanted it to be made clear what was happening, and the separation does seem to make sense. (Ideally attachment display settings gets broken up into subviews anyway.)

nacin9 months ago

nacin9 months ago

comment:12 nacin9 months ago

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

In 24777:

Media: Add awareness to Attachment Display Settings that audio and video can be embedded.

Also:

  • Add file length metadata to Attachment Details.
  • Round the kb/s bitrate on post.php.

fixes #24449.

comment:13 nacin9 months ago

Thanks koopersmith for helping with [24777].

comment:14 markjaquith9 months ago

  • Priority changed from normal to high
  • Resolution fixed deleted
  • Severity changed from normal to blocker
  • Status changed from closed to reopened

Blocker: this does not work for newly uploaded media. You have to reload the page first.

nacin9 months ago

nacin9 months ago

nacin9 months ago

comment:15 nacin9 months ago

  • Keywords i18n-change added

Decided to go with 24449.6.diff in IRC.

comment:16 nacin9 months ago

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

In 24784:

Media: Fix embedding of audio/video players when the file was just uploaded.

While uploading, we know an attachment's filename but not its mime type. Check specifically for extensions. Check for type as a convenience when it is available.

fixes #24449.

Note: See TracTickets for help on using tickets.