WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#24289 closed defect (bug) (fixed)

Post Formats UI: Attachment Display Settings ignored when inserting image.

Reported by: jbobich Owned by: azaozz
Milestone: 3.6 Priority: high
Severity: blocker Version: 3.6
Component: Administration Keywords: has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

I saw a couple tickets that are somewhat similar to this (#24175, #24147), and so I'm not sure if this is a bug, or maybe it just hasn't been addressed yet.

When inserting an image of an Image format post, none of the Attachment Display Settings seem to get inserted with the HTML of the image.

See screenshot: http://www.uploadblvd.com/uploads/image_518ab3ad0c9c4.png

Tested with:

  • WP 3.6-beta2-24163
  • Twenty Twelve theme, Twenty Thirteen theme
  • Chome 26, Firefox 20

Additionally, in digging through this process of how the image gets inserted, I could be mistaken, but it doesn't seem there is a way to filter this markup as it gets inserted, similar to what we have with the media_send_to_editor and image_send_to_editor filters. If I'm not missing the obvious, this might be a nice feature to consider incorporating.

Attachments (2)

24289.diff (958 bytes) - added by adamsilverstein 11 months ago.
insert only image URL when dragging/dropping or using modal to upload image
24289.2.diff (691 bytes) - added by kovshenin 11 months ago.

Download all attachments as: .zip

Change History (30)

comment:1 SergeyBiryukov11 months ago

  • Description modified (diff)

comment:3 SergeyBiryukov11 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:4 follow-up: markjaquith11 months ago

  • Priority changed from normal to high
  • Severity changed from normal to blocker

We also should be pulling the URL out into the separate field.

comment:5 kovshenin11 months ago

Alignment, link and size can no longer be changed for the image post format since [24226].

comment:6 follow-up: adamsilverstein11 months ago

verified that it is no longer possible to reproduce the bug . same applies to #24260. filter issue is another question.

the 'Attachment Display Settings' section is no longer present in the image post format media dialog reached from 'Select/Upload Image':

http://f.cl.ly/items/3S1Q2e2r0Y3D3f0v2i31/Screen%20Shot%202013-05-11%20at%2012.05.18%20PM.jpg

comment:7 in reply to: ↑ 6 ; follow-up: jbobich11 months ago

Replying to adamsilverstein:

the 'Attachment Display Settings' section is no longer present in the image post format media dialog reached from 'Select/Upload Image':

Yes, the attachment display settings are no longer visible, but I think that is only part of the issue. For me, the same HTML markup as before is being inserted into the text area.

I click to insert the image from the modal popup, and I get the image wrapped in an anchor tag linking to the enlarged version of that image (although I obviously have no settings to change this action when inserting the image).

Here's what I see after inserting an image: http://www.uploadblvd.com/uploads/image_518e8c8b0b05b.png

Possibly something to do with my previous settings the last time I inserted an image into a post? Either way, should probably be addressed. The end result on the frontend of Twenty Twelve and Twenty Thirteen for me is that my "Image click-through link" is ignored in favor of that link automatically wrapping the image. Can you reproduce this?

For reference, I've also pointed this out here, as well, with some additional thoughts on the subject.

Last edited 11 months ago by jbobich (previous) (diff)

comment:8 in reply to: ↑ 4 adamsilverstein11 months ago

Replying to markjaquith:

We also should be pulling the URL out into the separate field.

there is a separate 'Image click-through link' field on the image post format page, but the field seems to be ignored.

http://f.cl.ly/items/092v2P3c0p1F1K130z3P/Screen%20Shot%202013-05-12%20at%204.16.49%20PM.jpg

page html:
http://f.cl.ly/items/09453L0z0l011a2G2z3s/Screen%20Shot%202013-05-12%20at%204.19.42%20PM.jpg

comment:9 in reply to: ↑ 7 adamsilverstein11 months ago

Replying to jbobich:

Replying to adamsilverstein:

the 'Attachment Display Settings' section is no longer present in the image post format media dialog reached from 'Select/Upload Image':

Yes, the attachment display settings are no longer visible, but I think that is only part of the issue. For me, the same HTML markup as before is being inserted into the text area.

ok thanks for clearing up the rest of the issue, now I see the problem clearly - will dig a bit to see if i can figure out whats going on.

adamsilverstein11 months ago

insert only image URL when dragging/dropping or using modal to upload image

comment:10 follow-up: adamsilverstein11 months ago

  • Keywords has-patch dev-feedback reporter-feedback added

in 24289.diff:

  • insert only image URL when dragging/dropping or using modal to upload image
  • field is designed to take image URL, but modal or drag/drop action was inserting full html, caption code, link to image etc. as set for that image.

comment:11 in reply to: ↑ 10 ; follow-up: kovshenin11 months ago

Replying to adamsilverstein: The original HTML resulted in a faster image lookup in get_the_post_format_image and img_html_to_post_id because of the wp-image-* class. A lookup only by URL results in a database query, see attachment_url_to_postid. I think we should go back to using numeric IDs where possible.

comment:12 in reply to: ↑ 11 jbobich11 months ago

Replying to kovshenin:

Replying to adamsilverstein: A lookup only by URL results in a database query, see attachment_url_to_postid. I think we should go back to using numeric IDs where possible.

I agree with this. I think this was a little more confusing with the direction the image format was going, previously. However, now you have hidden the custom HTML block, and are just showing the image initially when inserted.

So, if possible, I think you should look at inserting the attachment ID (similar to setting a featured image), and maybe as a secondary thing, you can allow the more savvy user a chance to replace that with the custom block of HTML if they want (which the UI does at the now as of beta3).

Version 0, edited 11 months ago by jbobich (next)

comment:13 kovshenin11 months ago

I think inserting a custom HTML block for images is overkill, especially because of the fact that get_the_post_format_image will try to turn whatever markup there is into an attachment ID (which is extra work), and if it succeeds, everything else is thrown away (click-through links, sizes, classes, etc.) and the image is rendered with wp_get_attachment_image.

I'm not entirely sure, but I think HTML support was brought in for two reasons (someone can correct me):

  • Captions support
  • URLs support

For URLs, we can either sideload the image (brings the benefit of sizing and cropping) or just display as is (simpler and faster).

For captions it's slightly trickier. There used to be a dedicated caption field, just like a URL, but that's confusing, because there already is a working caption field in the media modal which inserts the [caption] shortcode.

The caption text is saved with the attachment post (not the regular post) so there's a benefit of inserting it into the editor (or meta) - you can have the same image twice but with a different caption. Otherwise a changed caption may affect existing posts, which is currently the case with galleries.

Since "attaching an image to a post" is no longer a key concept after the 3.5 media change, I think the caption should stay with the image, not the post, though that has other disadvantages too, like not being able to search the caption text.

Overall, I think it would be a good idea to remove the HTML textarea for image post formats, use numeric IDs or URL strings in meta, use the caption field to render captions, and bring back the "Insert from URL" menu in the media modal.

comment:14 follow-up: kovshenin11 months ago

Yes, the attachment display settings are no longer visible, but I think that is only part of the issue. For me, the same HTML markup as before is being inserted into the text area.

@jbobich can you still reproduce this with latest trunk?

comment:15 in reply to: ↑ 14 ; follow-up: jbobich11 months ago

Replying to kovshenin:

Yes, the attachment display settings are no longer visible, but I think that is only part of the issue. For me, the same HTML markup as before is being inserted into the text area.

@jbobich can you still reproduce this with latest trunk?

I'm actually still getting the same issue. I'm downloading the latest version in trunk, showing as 3.6-beta3-24280 in my WP admin, and clearing all browser cache.

Are you sure the change has made it in? In opening up /wp-admin/js/post-formats.js I can see where it's still inserting the image as before, opposed to just the attachment URL. I'm not seeing changes of 24289.diff.

Last edited 11 months ago by jbobich (previous) (diff)

comment:16 in reply to: ↑ 15 ; follow-ups: kovshenin11 months ago

Replying to jbobich: The image HTML is inserted, but any insert settings you may have saved, like alignment and link, should be discarded.

comment:17 in reply to: ↑ 16 adamsilverstein11 months ago

Replying to kovshenin:

Replying to jbobich: The image HTML is inserted, but any insert settings you may have saved, like alignment and link, should be discarded.

when an image is inserted this way (with HTML), the value entered into the 'Image click-through link' field is ignored. is that addressed in a separate ticket?

comment:18 in reply to: ↑ 16 ; follow-up: jbobich11 months ago

Replying to kovshenin:

Replying to jbobich: The image HTML is inserted, but any insert settings you may have saved, like alignment and link, should be discarded.

I'm not sure if it's a result from any saved settings, but I get the same as before. I get an image tagged wrapped in a link to the same image URL.

Screenshot: http://www.uploadblvd.com/uploads/image_5196852f54b78.png

Is that not what we're talking about here? That seems like a significant issue because if a user inserted an image, now putting anything into the "Image click-through link" won't work. Or is this a separate issue, as @adamsilverstein asked?

Last edited 11 months ago by jbobich (previous) (diff)

comment:19 in reply to: ↑ 18 ; follow-up: kovshenin11 months ago

Replying to jbobich: If the user manually inserts a link into the image HTML, then that link will take precedence over the one entered in the click through link, otherwise you'd get a link inside a link. However, this ticket was about the media modal inserting things into the HTML field, not users typing in HTML manually.

comment:20 in reply to: ↑ 19 ; follow-up: adamsilverstein11 months ago

Replying to kovshenin:

Replying to jbobich: If the user manually inserts a link into the image HTML, then that link will take precedence over the one entered in the click through link, otherwise you'd get a link inside a link. However, this ticket was about the media modal inserting things into the HTML field, not users typing in HTML manually.

when i drag an image or upload using the modal, i get HTML linking to an attachment. the only way to get the 'Image click-through link' to work is to manually enter an image URL into the field after switching to that mode.

it seems like i should be able to upload an image using drag and drop, enter a link into 'Image click-through link' and expect it to work.

it seems like i should open a new ticket for this at this point - seems like a separate issue; the issue described in this ticket is no longer valid: "Post Formats UI: Attachment Display Settings ignored when inserting image." - there are no longer Attachment Display Settings when uploading an image here

comment:21 in reply to: ↑ 20 kovshenin11 months ago

Replying to adamsilverstein: Ha, okay now I can reproduce it. Working on a fix. Thanks!

kovshenin11 months ago

comment:22 follow-up: kovshenin11 months ago

  • Keywords dev-feedback reporter-feedback removed

Here we go :) 24289.2.diff

comment:23 follow-ups: jbobich11 months ago

Phew, I thought I was taking crazy pills or something.

And apologies for the confusion, still sort of fumbling around, learning the formalities of trac.

comment:24 in reply to: ↑ 23 kovshenin11 months ago

Replying to jbobich: No need for apologies - I was the one taking crazy pills ;)

For reference, to reproduce this you need to make sure that the urlbutton user setting is set so something other than none. You can switch to the standard post format and insert an image with a link to save the user setting. As seen from the patch, we currently take that setting into account when calling wp.media.string.image during upload and select.

comment:25 in reply to: ↑ 22 adamsilverstein11 months ago

Replying to kovshenin:

Here we go :) 24289.2.diff

as expected, tested and it works perfectly - much better than my solution of chopping the entire (HTML) limb off :) thanks.

comment:26 in reply to: ↑ 23 adamsilverstein11 months ago

Replying to jbobich:

Phew, I thought I was taking crazy pills or something.

And apologies for the confusion, still sort of fumbling around, learning the formalities of trac.

completely normal for confusion to occur, often hard to grock issues other users describe, or what intention of code was/is.

your examples, screenshots and descriptions were very helpful and in the end we wound up with a useful patch that helps a lot.

thanks!

comment:27 follow-up: azaozz11 months ago

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

In 24289:

Post formats: when uploading an image or selecting it from the media modal, don't wrap it in a link, props kovshenin, fixes #24289

comment:28 in reply to: ↑ 27 kovshenin11 months ago

Replying to azaozz: Thanks!

Note: See TracTickets for help on using tickets.