Make WordPress Core

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#23964 closed defect (bug) (fixed)

Link url in Post Format Image doesn't link

Reported by: rdall's profile RDall Owned by: markjaquith's profile markjaquith
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Post Formats Keywords: has-patch
Focuses: Cc:

Description

I added a post format image and adding a website to the link url below. I published the post and tried to click the image and it doesn't link to the external website.

See attached screenshot: http://cl.ly/image/1S2B250e2z0Z

My post is here http://sandbox.robertdall.com/?p=1360 and I am running 3.6-beta1-23919

Attachments (5)

23964.diff (2.0 KB) - added by wonderboymusic 12 years ago.
23964.1.diff (598 bytes) - added by obenland 12 years ago.
Let image urls be obtainable through get_the_post_format_url()
23964.2.diff (2.5 KB) - added by markjaquith 12 years ago.
23964.3.diff (638 bytes) - added by wonderboymusic 12 years ago.
Don't double-link
23964.4.diff (1.1 KB) - added by wonderboymusic 12 years ago.

Download all attachments as: .zip

Change History (30)

#1 @Anderton
12 years ago

Tested on 3.6-beta1-23927 could replicate on Twentythirteen. However, it's working with the SemPress Theme (http://wordpress.org/extend/themes/sempress).

Last edited 12 years ago by Anderton (previous) (diff)

#2 @RDall
12 years ago

  • Component changed from Post Formats to Bundled Theme

I concur… Testing with SemPress the link does work.

#3 @obenland
12 years ago

  • Component changed from Bundled Theme to Post Formats
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.6

#4 @TravisHoffman
12 years ago

The difference between TwentyThirteen and SemPress is that SemPress displays the image using the_content() and TwentyThirteen uses the_post_format_image() which in turn calls get_post_format_image(). This function does not utilize the url from $meta. When I attempted to check for a non empty url in the meta data and surround the img tag with an a tag it ended up messing up the styles and the image would justify to the left.

I am new to submitting patches here and not sure what the best approach to fixing this would be. Is modifying get_post_format_image going to mess other things up that use it? So I opted to not submit a patch for that solution.

Is the correct course of action to correct the issue within the content-image.php file within the TwentyThirteen theme? Seems the more sensible solution as it seems to be a theme specific issue.

Comments?

#5 @jbobich
12 years ago

  • Cc jbobich added

#6 @wonderboymusic
12 years ago

  • Keywords has-patch added; needs-patch removed

23964.diff fixes this. get_the_post_format_image() now returns a linked image whenever the _wp_format_url value is un-empty. Image selected via the modal, image HTML inserted into the editor, image attached to the post: all work.

@obenland
12 years ago

Let image urls be obtainable through get_the_post_format_url()

#7 @jbobich
12 years ago

+1

Seems strange that in post_formats_compat the image is returned with the link if it exists, but if you've got support for structured-post-formats > Image format, and you're using the_post_format_image(), user's Link field is ignored.

... Or maybe if you're supporting the image structured post format, it's expected that you'd then be coming up with your own way to determine if the image should be wrapped in the link. Either way, as pointed out earlier, TwentyThirteen does completely ignore the Link field on an Image post, which doesn't seem right.

Last edited 12 years ago by jbobich (previous) (diff)

@markjaquith
12 years ago

#8 @markjaquith
12 years ago

Combined the patches, and cleaned up the sprintf stuff a bit.

#9 @markjaquith
12 years ago

Linking an image in 2013 makes the image huge. Some CSS issue. Looking into it.

#10 follow-up: @markjaquith
12 years ago

In 23991:

Constrain the width of images even when they're linked, in Twenty Thirteen.

see #23964

#11 @markjaquith
12 years ago

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

In 23992:

Link post format images if a URL is provided. Make the URL available via get_the_post_format_url().

props wonderboymusic, obenland. fixes #23964.

#12 in reply to: ↑ 10 ; follow-ups: @lancewillett
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to markjaquith:

In 23991:

Constrain the width of images even when they're linked, in Twenty Thirteen.

see #23964

Oh gosh, that's a super scary selector. O_o

I'd love to make it more concise.

Also, per WP CSS code standards, selectors should be on their own line.

#13 @lancewillett
12 years ago

@obenland is looking at making this too-general selector smarter. Per Twenty Thirteen office hours today in #wordpress-dev IRC.

#14 @obenland
12 years ago

I have a hard time creating a repeatable output for the selector to fit.

The image selectable in the "Select / Upload Image" box differs in size, based on what has previously been set in the Media modal for the main content. Also, if that image happens to have had a URL set with it, the custom URL doesn't work. Example output in my testing:

<a href="http://twitter.com/obenland"><a href="http://localhost:8888/trunk/wp-content/uploads/2013/02/cropped-MCM_4677.jpeg"><img src="http://localhost:8888/trunk/wp-content/uploads/2013/02/cropped-MCM_4677.jpeg" width="1500" height="375" alt="cropped-MCM_4677.jpeg" class="wp-image-1140 alignright size-full" /></a></a>

Or as a thumbnail with caption:

<a href="http://twitter.com/obenland"><div id="attachment_1082" class="wp-caption alignright" style="width: 160px"><a href="http://localhost:8888/trunk/wp-content/uploads/2013/03/13-MCM_0885.jpeg"><img src="http://localhost:8888/trunk/wp-content/uploads/2013/03/13-MCM_0885.jpeg" width="150" height="150" alt="with caption" class="wp-image-1082 size-thumbnail" /></a><p class="wp-caption-text">with caption</p></div></a>

@wonderboymusic: Is it intentional that the image preview reverts back to size selected after saving? This looks really odd with big pictures that happen to be in full-size.

@wonderboymusic
12 years ago

Don't double-link

#15 @wonderboymusic
12 years ago

  • Keywords commit added

23964.4.diff fixes a few things related to the new way that images are inserted into the meta field

#16 @tw2113
11 years ago

Not sure how related this is to this ticket, but it does relate to provided urls. When I use the Image post format and insert an image via the media gallery, it automatically wraps the image with a link to the media attachment url. I then provide a url in the input below the image, and that link is put out as well.

Result is an image wrapped in 2 different urls, invalidating markup and obeying the href attribute of the inner link.

Twentythirteen theme, trunk 3.6-beta1-24023

#17 follow-up: @markjaquith
11 years ago

The linking bug tw2113 mentioned is still occurring with .4.diff. As is the huge image size bug obenland mentioned.

#18 in reply to: ↑ 17 @SergeyBiryukov
11 years ago

Replying to markjaquith:

The linking bug tw2113 mentioned is still occurring with .4.diff.

There's a similar bug in post_formats_compat(), see #24147.

#19 @wonderboymusic
11 years ago

  • Keywords commit removed

there is a fistful of confusing media library code I am wading through now - figuring out how to obtain "settings" when the "select" event fires

Last edited 11 years ago by wonderboymusic (previous) (diff)

#20 @markjaquith
11 years ago

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

In 24066:

Multiple improvements to image post format insertion and display.

  • get_tag_regex() altered based on Unit Tests.
  • Changes to post-formats.js to provide size and link context during image selection.
  • Captions are now output in the_post_format_image() when present.
  • The meta value for url is respected for the image post format when the HTML in the image meta doesn't include a link

props wonderboymusic. fixes #23965, #23964. see #24147, #24046.

#21 in reply to: ↑ 12 @SergeyBiryukov
11 years ago

Replying to lancewillett:

Oh gosh, that's a super scary selector. O_o

I'd love to make it more concise.

Follow-up: #24198

#22 @SergeyBiryukov
11 years ago

In 24118:

Fix typo in get_the_post_format_image(). props rlerdorf. see #23964. see #24210.

#23 in reply to: ↑ 12 @lancewillett
11 years ago

Replying to lancewillett:

Replying to markjaquith:

In 23991:

Constrain the width of images even when they're linked, in Twenty Thirteen.

see #23964

Oh gosh, that's a super scary selector. O_o

I'd love to make it more concise.

Fixed in r24159.

#24 follow-up: @klihelp
11 years ago

When you select the image for the image post format:
the Media popup ATTACHMENT DISPLAY SETTINGS is hidden, but the popup still remembers the last used url settings (none, media file, attachment ...)

Version 5, edited 11 years ago by klihelp (previous) (next) (diff)

#25 in reply to: ↑ 24 @SergeyBiryukov
11 years ago

Replying to klihelp:

the Media popup ATTACHMENT DISPLAY SETTINGS is hidden, but the popup still remembers the last Link To setting (none, media file, attachment ...) and the link is added to the image.

Related: #24289

Note: See TracTickets for help on using tickets.