#26642 closed task (blessed) (fixed)
HTML5 captions
Reported by: | Looimaster | Owned by: | azaozz |
---|---|---|---|
Milestone: | 3.9 | Priority: | normal |
Severity: | normal | Version: | 3.8 |
Component: | Media | Keywords: | has-patch |
Focuses: | Cc: |
Description
In /wp-includes/media.php in img_caption_shortcode()
there is a note saying By default, the caption is 10 pixels greater than the width of the image, to prevent post content from running up against a floated image.
but this makes the caption text misaligned (too long) 10px in all the possible cases:
- image floating left
- image floating right
- image centered
- image with
alignnone
Caption text is longer than the image. Not exactly below the image.
Furthermore, if an image without caption (for example 150px wide) is placed below/above the image with caption (150px converted to 160px) and both are left aligned then it forms a misaligned grid (all images with captions push the text 10px to the right).
When I remove this 10px increment, everything looks exactly like expected. Post content doesn't run onto the image or caption in any case and I tested them all. I'm wondering if this 10px increment should be there? What does it do? Maybe that's a bug?
Attachments (19)
Change History (59)
#2
@
11 years ago
- Cc bcworkz added
- Keywords has-patch 2nd-opinion added
- Type changed from defect (bug) to enhancement
I agree the extra 10 pixels added to the caption content container was a poor decision. Such issues as post content running into caption text is supposed to be dealt with by CSS and themes. But it's been this way for at least 6 years and was a conscious decision, so we can't call it a bug. I'm changing it to enhancement request, as I would like to see the 10 px removed from the caption on principle, even though filtering it to one's desire is super easy.
The issue though is most decent themes have compensated for this, so removing the 10 px will break many of the themes (depending on how they compensated in the first place) that were most carefully crafted to ensure the content lined up. It's a no win enhancement. But conceptually, the separation of content and style dictates this spacing is in the wrong place and should be removed. Thus, I'm providing a patch even though I'd be surprised if this gets far.
#4
@
11 years ago
- Keywords has-patch 2nd-opinion removed
- Milestone Awaiting Review deleted
- Resolution set to invalid
- Status changed from new to closed
bcworkz, we can't remove the default value for backward compatibility reasons.
Looimaster, you can use the filter (added in [25444]) to remove the 10px (see below) and add your own styles to make it pretty in your theme.
add_filter( 'img_caption_shortcode_width', '__return_zero' )
#7
@
11 years ago
- Resolution invalid deleted
- Status changed from closed to reopened
ocean90:
WordPress 3.8 introduced MP6 that broke admin panel designs of a lot of plugins and themes. Many needed an update of styles. Many still don't work with this new skin.
WordPress 3.9-alpha switched from TinyMCE 3.x to 4.x, breaking 95% of TinyMCE plugins overnight.
So, that is allowed and fixing this little issue is not allowed?
An issue that is troubling many developers has been identified and has a patch now. I think it should be fixed right now because a lot of people are puzzled why this happens and it produces invalid look of images unless a theme adds some workarounds in style.css.
sonic1243: Agree, it has to change someday or more people will be entering bug tracker and reporting the same issue.
#9
follow-up:
↓ 10
@
11 years ago
- Keywords close added
Causing breakage in the admin is a far cry from breaking the theme's design.
#10
in reply to:
↑ 9
@
11 years ago
Replying to nacin:
Causing breakage in the admin is a far cry from breaking the theme's design.
My point was simply that we have a work-around for the public site, but there is no work-around for the editor without editing the core files directly?
This bug/error belongs in the history books ... Code is poetry...
This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.
11 years ago
#12
@
11 years ago
In 26642.3.diff: tweaked 26642.2.diff a bit to avoid JS type juggling in wpeditimage
(we got burned by that :) ).
#13
@
11 years ago
- Keywords close removed
Introduce HTML5 support for captions, conveniently ignoring the 10px issue.
Related: #27292
#14
@
11 years ago
Hello, jumping in from #27292 to this hijacked ticket ;). Seems like obenland and I was thinking about captions and HTML5 about the same time. My two cents on various topics:
About adding an inline style width + 10px: As ocean90 pointed out (#comment:4) this can be removed on the front-end using the filter img_caption_shortcode_width
. This filter does not work in the visual editor though (?). Uncertain if it is possible to get it working there.
Obenland suggests that by adding a new html5 theme support 'caption', the markup should change to use <figure> and <figcaption> elements and skip the inline width style altogether. Also bypassing the img_caption_shortcode_width
filter. One could argue that this special treatment of HTML5-supported captions is not fully logical. Removal of an inline style is unrelated to using HTML5 markup.
Using an inline width style for an image with a caption can be useful. For example, say that you have a small (narrow) image with a fairly long caption. Then using an inline width style that is wider than the image can make things look better, as there will be fewer line breaks in the caption. As the image width might be unknown to the style sheet, using an inline style might be warranted.
The patch in #27292 also utilizes the 'caption' html5 support to get <figure> and <figcaption> elements but does not change the treatment of the inline style.
In case the approach in the current thread is adopted, the unit test provided in #27292 can still be used. (Though it could be tweaked to explicitly check for the presence of a style attribute if desired).
#15
@
11 years ago
This is a bit crazy idea I got, but it would solve the original inline style problem without mixing in HTML5 support.
Patch 26642.4 borrows heavily from 26642.3, but instead makes use of a new theme support 'no-inline-style-captions'.
#16
@
11 years ago
The added 10px of width have been confusing to theme developers for a while now. The introduction of the shortcode_atts_*
filter made that a little better (in a hacky way), but hasn't really resolved that.
With the the switch to HTML5 we finally have the opportunity to stop the madness. You can see declaring HTML5 support as a switch of context for the individual feature, like galleries that won't output default styles when the theme added support for HTML5 gallery markup (r27396).
#17
@
11 years ago
Given that the inline width is there for legacy reasons, and that introducing yet-another-argument for theme support means maintaining it into the future (and isn't really any more dev-friendly than the existing filter), I think dropping it with the introduction of HTML5 support is a perfectly sane route. It's not any different from what was done with galleries.
#18
@
11 years ago
Also the current patches here remove the style="width..."
from the wrapper element. That would make it impossible to center the caption wrapper (the image can be centered but the caption text would be centered too) and the text won't wrap around under the image when longer. Is that intended for the HTML 5.0 captions?
#19
@
11 years ago
I think azaozz has a point here. I did some more testing with short and long caption text, using the default styling of the wrapper element compared to no inline styling. In both cases I used the default editor styling of the captioned image. See the attached images. Obviously there are consequences of skipping the inline style, desired or not.
Additionally I found a few more places in wpeditimage/plugin.js
where the style="width: +10px"
was added (e.g. when editing an image). The attached new patch .6 accounts for this.
#20
@
11 years ago
Note that in the above attached Default style.png
image my intention was to change the caption text to "default-style-caption" or something similar. You have to imagine that. Sorry for the inconvenience.
#21
follow-up:
↓ 23
@
11 years ago
Sorry for all the spam here, but I just thought about another option that maybe azaozz was looking for: Keep the inline style but don't add 10 px. The image below shows how that looks in the editor.
To accomplish that I introduced a theme support called 'no-add-10px-captions'
, but that could of course be called something else. I don't want to introduce HTML5 markup here, since I think that belongs to #27292 (though of course I'm biased here).
Update: There is now a new patch in #27292, that removes the by-default added 10 px, as well as introduces HTML5 markup.
#22
@
11 years ago
- Milestone changed from Awaiting Review to 3.9
Replying to azaozz:
Also the current patches here remove the
style="width..."
from the wrapper element. That would make it impossible to center the caption wrapper […] Is that intended for the HTML 5.0 captions?
Themes can use display: table;
on centered captions. While this doesn't work for old versions of IE, HTML5 in general doesn't work there either.
I added mce styles to make HTML5 captions look as close to its HTML4 predecessors as possible. Example
#23
in reply to:
↑ 21
;
follow-ups:
↓ 24
↓ 35
@
11 years ago
Replying to jond3r:
I just thought about another option that maybe azaozz was looking for: Keep the inline style but don't add 10 px.
Yes, exactly.
Replying to obenland:
Themes can use
display: table;
on centered captions.
Ah, the "ancient" layout table. Yes, this solves the centered caption problem (as long as themes are OK with being incompatible with some browsers), but the bigger problem of wrapping caption text under the image still remains.
Thinking we should do exactly what this ticket was opened for: remove the extra 10px but keep the inline stile as there's no good way to wrap the text under the image without it.
#24
in reply to:
↑ 23
@
11 years ago
Replying to azaozz:
Thinking we should do exactly what this ticket was opened for: remove the extra 10px but keep the inline stile as there's no good way to wrap the text under the image without it.
We came so close. But you're right, I can't think of a better way than keeping the style attribute around either.
#25
@
11 years ago
In 26642.9.diff:
We still use an inline style in the HTML5 version to encourage captions to break. The width is not filtered since the original purpose of the existing img_caption_shortcode_width
filter was to give theme authors a way to circumvent the added 10 pixels.
On the editor side we added a flag to signal html5 caption support. Based on that flag the editor now adds those 10px (or not), and gives captions a general 4px padding (or not).
#26
follow-up:
↓ 29
@
11 years ago
In 26642.10.diff, tweak 26642.9.diff a bit and add checks for wpeditimage_html5_captions
in few more places.
Thinking we should be bolder in the default style for html5 captions inside the editor. Perhaps remove the background, margin, padding, and container border, etc.
#27
@
10 years ago
- Summary changed from [caption] is 10px too wide to HTML5 captions
- Type changed from enhancement to task (blessed)
#29
in reply to:
↑ 26
@
10 years ago
Leaving this open so we could look into this:
Replying to azaozz:
Thinking we should be bolder in the default style for html5 captions inside the editor. Perhaps remove the background, margin, padding, and container border, etc.
#30
@
10 years ago
"Perhaps remove the background, margin, padding, and container border, etc."
Yes please! I'm having to add css to override the default tinymce theme's caption's border/background, also the alignnone/left/right/center classes, and even the font in tables (my unanswered forum post a few days ago here: http://wordpress.org/support/topic/tinymcethemes-css-overriding-my-editor-style?replies=1). Hope I've understood you correctly... perhaps editor styles aren't often taken as seriously as I am taking them?!
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
10 years ago
#34
@
10 years ago
- Keywords has-patch added
Patch removes basic styling and brings captions in sync with how non-captioned images are displayed.
Before and after: https://cloudup.com/cDOLFPmKdqr
#35
in reply to:
↑ 23
@
10 years ago
but the bigger problem of wrapping caption text under the image still remains.
you may want to try with display: table-caption;
and caption-side: bottom;
applied to the caption, it will work as long as the caption container will have `display: table;
Themes can use the same solution. Added bonus: just switching to caption-side: top;
will display the caption on top.
see http://jsfiddle.net/6XZkN/
attached a .zip with a simple html to be tested on a local server
#36
@
10 years ago
- Owner set to azaozz
- Status changed from reopened to assigned
Final tweaks need review.
#38
follow-up:
↓ 40
@
10 years ago
When HTML 5 captions are enabled, the editor still uses dd
and dt
tags, instead of figure
and figcaption
. What is the reason for this? It seems to me like the tags should be consistent across the editor and front-end.
Sorry to add to this thread, but I couldn't find the answer to this question here.
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
10 years ago
#40
in reply to:
↑ 38
@
10 years ago
Replying to ScottSmith:
Inside the editor the DL node is used in managing the caption. It has always been different from the tags outputted on the front-end. Having different nodes for html4 and html5 captions would double that code :)
Related: #14380, #17044.