Make WordPress Core

Opened 13 years ago

Closed 11 years ago

Last modified 4 years ago

#17044 closed defect (bug) (duplicate)

div.wp-caption shortcode should have width of img, rather than add +10px

Reported by: mgk25's profile mgk25 Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Shortcodes Keywords:
Focuses: Cc:

Description

In WordPress 3.1, file wp-includes/media.php function img_caption_shortcode() ends with the line

return '<div ' . $id . 'class="wp-caption ' . esc_attr($align) . '" style="width: ' . (10 + (int) $width) . 'px">'
[...]

This line unfortunately hardwires into WordPress (apparently since version 2.6) a style-sheet issue, namely it makes the <div> that combines an image with its caption text 10 pixels wider that the image itself.

Why, and why exactly 10px?

This seems a bad idea for several reasons:

  • The addition of a caption needlessly changes horizontal spacing around a floating image by 10px, which looks awkward if a posting includes several left-floating images, some of which have captions and some of which do not (and therefore use no caption shortcode). Those images that have captions will have 10px additional space compared to those that do not, leading to an uneven appearance that can't be fixed neatly in theme CSS.
  • Any desired additional space could easily be offered in the theme CSS via a margin, and there is no need to hardwire this into the width, Such stylistic choices (such as 10px) should really be left to the theme CSS, rather than hardwired into WordPress.

Proposed patch:

Please replace in the above line the term

(10 + (int) $width)

with

((int) $width)

in other words remove the hardwired additional width for the img+caption containing div.

Users have complained about this issue many times in the support forum, and it clearly causes a lot of confusion and unexpected behaviour, but I haven't seen it yet filed as a bug:

http://wordpress.org/support/topic/10px-added-to-width-in-image-captions

http://wordpress.org/support/topic/attached-image-captions-adding-10px

http://wordpress.org/support/topic/wp-caption-how-to-set-the-right-and-left-padding-for-captions

http://wordpress.org/support/topic/control-of-image-caption-width

http://wordpress.org/support/topic/should-i-edit-mediaphp

A related complaint:

http://core.trac.wordpress.org/ticket/9066

Attachments (2)

caption-width.png (50.6 KB) - added by azaozz 12 years ago.
no-width-caption.png (52.4 KB) - added by azaozz 12 years ago.

Download all attachments as: .zip

Change History (22)

#1 @scribu
13 years ago

I agree that .wp-caption {padding: 5px} would probably work better.

I assume it was added so that theme authors wouldn't have to add that rule themselves.

For reference: #6812

#2 @greenshady
13 years ago

This is one of the things I hate the most when styling captions. But, if we change it now, it'd break a ton of themes that have styled around this issue.

One can always filter img_caption_shortcode though.

#3 @scribu
13 years ago

  • Keywords close added

#4 @styledev
12 years ago

  • Cc styledev added
  • Keywords 2nd-opinion added

I see where you are coming from Justin, but I do not think that should be an excuse to not fix this. That is why we deprecate things and inform people and then eventually take out deprecated code. Everyone who is using the deprecated code will have to fix their themes to use the new code just like they will have to add CSS in to fix this.

Understandably though there is no good way to deprecate this but I believe that Wordpress should leave designing to the designers.

#5 follow-up: @azaozz
12 years ago

Seems many people dislike the width on that wrapper div. As far as I remember it was added so captions can be centered. Haven't seen any suggestions on what to replace it with or how to handle centered captions.

#6 in reply to: ↑ 5 ; follow-up: @greenshady
12 years ago

Here's a fix you can add to your themes:
http://svn.locallylost.com/themes/hybrid-core/trunk/extensions/cleaner-caption.php

Replying to azaozz:

Seems many people dislike the width on that wrapper div. As far as I remember it was added so captions can be centered. Haven't seen any suggestions on what to replace it with or how to handle centered captions.

The problem is the additional 10px added to the width, not with having a width.

Although, I wouldn't mind seeing some solutions for removing the width altogether. What about using <figure>? I don't think you need a width on it, but I haven't really tested it to see how well browsers handle it.

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

Replying to greenshady:

The 10px are there to allow adding border to the wrapper so captions on the front-end look similar to captions in the visual editor. That was long before themes started to style the visual editor, even before browsers were supporting stuff like box-sizing: border-box; etc.

Frankly I don't see why the extra 5px on each side of the image are such a problem. It's not necessary to add border to the wrapping div or the padding can be adjusted after taking the 5px into consideration. Themes that want to have full control can replace the whole [caption] shortcode rendering, adding/removing/modifying anything they like.

#8 in reply to: ↑ 7 @greenshady
12 years ago

The reason it's a problem is that it makes it harder to style when you don't want borders or padding. It's a lot easier to add 5px of padding than to remove 10px of overall width from an element with CSS.

I still recommend closing this ticket though. Changing this would break many themes, there's no good way to deprecate the code, and it's easy to fix.

#9 follow-up: @oncletom
12 years ago

This 10px is just a total nonsense: WordPress core code should not provide bad hints to theme developers.

Here are some thoughts to satisfy legacy code (which is there since 3.1, it means many themes did not have this 10px in mind BEFORE 3.1 – breaking things since this time does not seem to be problematic though):

  • switch img_caption_shortcode as a reference filter
  • if it's impossible, adding a img_caption_shortcode_attributes filter after shortcode_atts (and extract later)

With one of these solutions:

  • attributes could be overloaded without rewriting the whole output
  • WordPress could embed a default filter to emulate this 10px and remove it in a certain number of release (deprecating right now this arbitrary number)
  • theme and developers could just REMOVE this filter to avoid this +10px width

Something like this:

function img_caption_shortcode($attr, $content = null) {
...
	$attr = shortcode_atts(array(
		'id'	=> '',
		'align'	=> 'alignnone',
		'width'	=> '',
		'caption' => ''
	), $attr);
	$attr = apply_filters('img_caption_shortcode_attributes', $attr);
	extract($attr);
...
}

//in default-filters.php
//@deprecated in wordpress 3.6
add_filter('img_caption_shortcode_attributes', function($attr){
	if (isset($attr['width']) && (int)$attr['width'] > 0){
		$attr['width'] += 10;
	}

	return $attr;
});

#10 @grahamarmfield
12 years ago

I'm liking oncletom's solution. It seems to satisfy the need for keeping existing themes sweet but allowing theme developers to remove it if they want to - and in such a way that the changes survive WP version upgrades.

#11 @nacin
12 years ago

I would like to try to remove it outright — what are the possible ramifications of this for themes? Screenshots and specific theme references are helpful. I am curious if more modern themes have enough CSS styling images and captions to make it gracefully work without the +10px in place.

#12 in reply to: ↑ 9 ; follow-ups: @azaozz
12 years ago

Replying to oncletom:

... (which is there since 3.1, it means many themes did not have this 10px in mind BEFORE 3.1 – breaking things since this time does not seem to be problematic though)

The extra 10px have been there since the introduction of image captions in 2.6, four years ago :)

Still haven't seen a single case where removing these 10px would make styling images with captions any easier/better, or even any different. Removing the width completely will make it easier for floated images, however there doesn't seem to be a straightforward way to center a caption without it.

Version 0, edited 12 years ago by azaozz (next)

#13 in reply to: ↑ 12 @grahamarmfield
12 years ago

Replying to azaozz:

Still haven't seen a single case where removing these 10px would make styling images with captions any easier/better, or even any different. Removing the width completely will make some difference for floated images, however there doesn't seem to be a straightforward way to center a caption without it.

As a theme developer it's always easier to style something that doesn't have inline styles already added. And to take an example from a design I'm doing for a client right now there is a requirement for images with captions to have a fine border round the caption and the image together but with no padding. I have had previous projects as well where the extra 10px has been a real pain.

As far as I can tell, removing the style="width: nnn" from the wp-caption div seems to make no difference to the display - even if it's floated left or right. It's possible to centre the caption itself by placing text-align:center on wp-caption div in your stylesheet. Or does this conflict with older IE versions? If so, maybe targeting the actual caption paragraph could be done.

#14 in reply to: ↑ 12 @oncletom
12 years ago

Replying to azaozz:

Replying to oncletom:

... (which is there since 3.1, it means many themes did not have this 10px in mind BEFORE 3.1 – breaking things since this time does not seem to be problematic though)

The extra 10px have been there since the introduction of image captions in 2.6, four years ago :)

Oh come on, please provide me pros and cons but not such a foolish thing. It has been around since 4 years and so what? If it had been bad for 4 years, is it a reason to keep it?

This property being inlined is very hard to override because it is inlined.

Still haven't seen a single case where removing these 10px would make styling images with captions any easier/better, or even any different. Removing the width completely will make some difference for floated images, however there doesn't seem to be a straightforward way to center a caption without it.

I don't ask to remove the width. Just about removing the extra 10 pixels of the real width.

If other people did not complain does not justify the fact *things are good, we don't touch them even if this is the matter of this issue*.

Seriously.

@azaozz
12 years ago

#15 @azaozz
12 years ago

Replying to nacin:

I would like to try to remove it outright...

Don't think that would be possible. In Twenty Eleven:


A lot of (most?) themes have adapted the captions styling to include the 10px. Not sure having a filter for it would be good either. Perhaps another add_theme_support?

add_theme_support( 'expanded-captions-width', array('expand' => 0) );

Replying to grahamarmfield:

... it's always easier to style something that doesn't have inline styles already added.

Agreed. How would you style a centered caption in lets say Twenty Eleven without the inline width? Unfortunately the browsers (and CSS) are not good in placing images in the center. It's quite hard to add an image in the center of a text block (surrounded by text) for example, and have the text flow nicely around it.


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

#16 @oncletom
12 years ago

I give up on this issue: I pasted code 7 months ago with a proper solution to avoid breaking compatibility, the initial problem talks about the 10 pixels extra width and not *removing the width from styles*.

If a picture is 460 pixel, the idea is to have style="width: 460px" instead of style="width: 470px".

Code is provided, explainations are provided.
Can't do more.

#17 @anjacob
12 years ago

  • Version changed from 3.1 to 3.4.1

I just ran into this bug. this is truly annoying problem, adding inline styles is always bad. All styling should be done using CSS.

#18 @helenyhou
12 years ago

  • Version changed from 3.4.1 to 3.1

Version number indicates when the bug was initially introduced/reported.

#19 @helen
11 years ago

  • Keywords close 2nd-opinion removed
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Closing as a duplicate of #14380, which has a patch.

#20 @derevco
4 years ago

My huge thanks to the author of this ticket! You helped me a lot.

Note: See TracTickets for help on using tickets.