Default link target for media files is none

[Clorith]

in r33729 the default link target for images was changed to None, which is all fine and dandy, but this also affects other media elements such as documents, PDF files and so forth.

Normal files should still link to the media file as default, as it's not helpful to the users to just have a file name inserted when they are inserting an actual file.

[SergeyBiryukov]

Normal files should still link to the media file as default, as it's not helpful to the users to just have a file name inserted when they are inserting an actual file.

Seeing similar reports on support forums as well.

[jorbin]

@liljimmi, @janhenckens, @eherman24, @wonderboymusic - This seems to have been introduced in [33729] which you all worked on, can you please take a look.

[eherman24]

Set default 'Link To' parameter to 'Attachment Page' for certain attachment mime types

[eherman24]

Set default 'Link To' parameter to 'Media File' for certain attachment mime types

[eherman24]

I've attached a first patch here. Sorry for posting two patches. The first one was linking to the attachment page, where it should have been linking to the actual media file itself.

I've gone ahead and added a check when a media element is selected in the media library. If the selected media file's mime type is found in the array, the 'Link To' for the element sets to 'file' (Media File).

Some additional feedback is needed, as I've defined a new array of default mime types to set the 'Link To' parameter to 'File'.

var mimeTypeDefaultLinkToAttachmentPage = [
   'pdf',
   'csv',
   'zip',
   'gz|gzip',
   'rar',
   '7z',
   'doc',
   'docx',
];

I got a list of acceptable mime types from the codex article get_allowed_mime_types() (​https://codex.wordpress.org/Function_Reference/get_allowed_mime_types), under 'Default allowed mime types'.

I'm sure additional mime types should be added to this list, but I'm not entirely sure which is best to have on this list and which is best left off.

[Clorith]

I'm thinking it would be safer to reverse that check?

If the mime is image/* then link to None, but if it isn't, link to the media file, as you are more likely to have an infinite amount of file variations than you are image types.

[eherman24]

Media file whos mime types != image/* default link to 'Media File'

[eherman24]

@Clorith Yea, I think that does make a bit more sense. Moving forward it's probably best not to have an array of file types to maintain.

The new patch checks the mime type and for anything not matching 'image/' will default to 'file' - or 'Media File' in the media library 'Link To' dropdown.

[janhenckens]

@Clorith @eherman24 Yeah, only setting it to none for images is a saner approch. Tested the patch against 4.4, looks good.

Do we want to revert anything from #31467 or can that stay in? (the goal there was to not link images by default and we ended up not linking anything by default)

[jorbin]

I'm torn on this since it sounds like the image_default_link_type option isn't going to be respected. Perhaps there needs to be a second option for other uploads?

@eherman24 thanks for the patch. In the future, please try to generate the patch from the root of your checkout. Can you refresh it please?

@melchoyce would love your UX thoughts here since you were a champion of the original change.

[eherman24]

Regenerated patch from root checkout

[eherman24]

@jorbin Sorry about that. It's been a little bit since I created a patch. I've gone ahead and regenerated the patch from the root of my install.

[SergeyBiryukov]

#35247 was marked as a duplicate.

[azaozz]

Replying to jorbin:

Perhaps there needs to be a second option for other uploads?

Yeah, it's logical to have different link options for images and other uploads. The default workflow is different:

• Images are better not wrapped in a link, see #31467.
• All other files should link to the file (the old behaviour), or optionally to the attachment page.

In addition this is affected by letting the user settings override the options, see #35101. Even if we change the default in image_default_link_type, the user can set it to none and still trigger the above error when inserting attachments that are not images.

For 4.4.1 we need to ensure the default cannot be none for non-image attachments. This should still respect the image_default_link_type option when changed by a plugin.

[azaozz]

In 35153.5.diff:

• Check if an attachment is image and if not ensure the link is set to something else than 'none'.
• Introduces wp.media.controller.Library.isImageAttachment().

While testing this discovered another bug: changing the settings object in defaultDisplaySettings() changes this._defaultDisplaySettings as it is a reference to it. So clicking on "embeddable" attachment sets this._defaultDisplaySettings.link = 'embed' which is used for all other attachments that are clicked after that. Had to clone this._defaultDisplaySettings to keep it unchanged.

[nacin]

LGTM, @azaozz.

[azaozz]

Media: when inserting an attachment in the editor and it is not an image, ensure the link is set to something else than none.

Props eherman24, azaozz.
Fizes #35153 for trunk.

[azaozz]

Reopen for 4.4.

[azaozz]

In 36167:

Media: when inserting an attachment in the editor and it is not an image, ensure the link is set to something else than none.

Props eherman24, azaozz.
Fixes #35153 for 4.4.1.