WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#6380 closed defect (bug) (wontfix)

the_excerpt & [gallery]

Reported by: DD32 Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.5
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

Having a post such as:

Source: http://...

[gallery]

and using the_except() will produce a post such as:

<p>Source: http://...</p>
<p>			.gallery {<br />
				margin: auto;<br />
			}<br />
			.gallery-item {<br />
				float: left;<br />
				margin-top: 10px;<br />
				text-align: center;<br />
				width: 33%;			}<br />
			.gallery img {<br />
				border: 2px solid #cfcfcf;<br />
			}<br />
			.gallery-caption {<br />
				margin-left: 0;<br />
			}</p>

Looks like the_excerpt should de-register the shortcodes the_content filter first, trim it, apply the shortcodes to the trimmed except, and then re-attach the shortcodes filter?

Attachments (4)

6380.diff (861 bytes) - added by DD32 6 years ago.
6380.2.diff (884 bytes) - added by DD32 6 years ago.
6380.3.diff (1.3 KB) - added by azaozz 6 years ago.
6380-style.diff (2.2 KB) - added by andy 6 years ago.

Download all attachments as: .zip

Change History (19)

DD326 years ago

DD326 years ago

comment:1 DD326 years ago

  • Keywords has-patch added; needs-patch removed

Traced it back to wp_trim_excerpt() which fakes a except if need be.

attachment 6380.diff added.

is one solution, yet slightly ugly.

attachment 6380.2.diff added.

is another, However, chances breaking HTML tags if it breaks half way through a html tag with attributes. The balance tags is there incase it breaks on '<b> 55th-word[...]'

strip_tags() could be run before it trims the word count, that'd help the chances of breaking html.

Also, Shouldn't the_except work with <!--more--> if it exists in the post?

azaozz6 years ago

comment:2 azaozz6 years ago

  • Milestone changed from 2.7 to 2.6

Following brief discussion with DD32, 6380.3.diff will strip all tags properly. The function strip_all_tags() will probably be usefull in other cases too.

However why would we replace [gallery] with a lot of html and then strip all of it? For now, there's no way to remove the shortcode if the function that replaces it is disabled.

andy6 years ago

comment:3 lloydbudd6 years ago

  • Milestone changed from 2.6 to 2.5.1

Setting to 2.5.1 based on markjaquith setting dup #6496 to this milestone.

comment:4 follow-up: tellyworth6 years ago

One issue that's related to the stylesheet location is that in 2.5.0, you can't have multiple galleries on one page with different numbers of columns. It looks like Andy's patch will fix this.

I'd advocate serving up a static external stylesheet rather than making it dynamic. Though it would initially have only the gallery styles in it, the static stylesheet could eventually contain default styles for other built-in shortcodes and content-related things.

Seems to me that the cost of scanning forward for [gallery] tags and conditionally including an inline stylesheet is out of proportion to the (minor) benefit of not serving up the gallery stylesheet on pages that don't use it.

As for the_excerpt: what about a toggle or wrapper function that would temporarily disable shortcode parsing and just return the literal "[gallery]" instead?

comment:5 in reply to: ↑ 4 ; follow-up: westi6 years ago

Replying to tellyworth:

Seems to me that the cost of scanning forward for [gallery] tags and conditionally including an inline stylesheet is out of proportion to the (minor) benefit of not serving up the gallery stylesheet on pages that don't use it.

I would prefer if a publish/save post hook set a post meta variable for the posts that said they contained a gallery shortcode that way a single query should be able to find out if we are going to display a gallery for the current page I think.

comment:6 in reply to: ↑ 5 markjaquith6 years ago

Replying to westi:

I would prefer if a publish/save post hook set a post meta variable for the posts that said they contained a gallery shortcode that way a single query should be able to find out if we are going to display a gallery for the current page I think.

Could scan for all known shortcodes. store as postmeta rows: uses_shortcode => gallery uses_shortcode => foo

then could have a contains_shortcode() function that could optionally accept a shortcode param.

<?php
if ( contains_shortcode( 'gallery' ) {
// do gallery foo bar
}

?>

But that's too ambitious for a 2.5.1 milestone.

comment:7 markjaquith6 years ago

Note that [6581] (trunk) and [6594] (2.5.1) fix the wpautop() mangling. But style still doesn't belong down here. Yeah, the lookahead isn't elegant, but it's quick and dirty compared to a custom field-based solution. I don't like including the style on every page -- I can see people being annoyed by that. I'd only want to resort to that if the gallery scan is very computationally expensive.

We can implement a more elegant solution for 2.6 or 2.7

comment:8 Denis-de-Bernardy6 years ago

I looked into the 3rd diff. there are occasions where the look ahead will never occur if you use the template_redirect hook as is. priority should be -1000 or something. and even then, sometimes, it won't get caught as expected.

with now reading, for instance, it wouldn't work -- it's arguably a non-issue for that plugin, but the point is to highlight that some plugins mangle wrongly or even skip template_redirect. this was among the reasons I had opened a ticket to request an ob_start hook at one point (which the devs refused, but that's another matter).

comment:9 Denis-de-Bernardy6 years ago

simpler fix:

# shortcodes
add_filter('get_the_excerpt', 'strip_shortcodes', 0);
add_filter('get_the_excerpt', 'restore_shortcodes', 20);
	
function strip_shortcodes($in)
{
	remove_filter('the_content', 'do_shortcode', 11);
	return $in;
} # strip_shortcodes()
	
	
function restore_shortcodes($in)
{
	add_filter('the_content', 'do_shortcode', 11);
	return $in;
} # restore_shortcodes()

comment:11 fuggi6 years ago

Also related to this ticket: http://trac.wordpress.org/ticket/4679
"the_content() vs. the_excerpt() - apply different filters to posts without excerpt text specified"

comment:12 ryan6 years ago

related #7100

comment:13 DD326 years ago

related #7100

Thanks for #7100, Shortcodes are stripped from Excerpts. It basically fixes this problem, However, Prevents Galleries in excerpts.

comment:14 ryan6 years ago

  • Milestone changed from 2.5.2 to 2.9

Milestone 2.5.2 deleted

comment:15 DD325 years ago

  • Milestone 2.9 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Closing this ticket as wontfix, Since Shortcodes are no longer in excepts, theres nothing left in this ticket.

Note: See TracTickets for help on using tickets.