WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 4 weeks ago

#19550 reopened enhancement

Please provide option to disable wptexturize entirely

Reported by: jwz Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 3.3
Component: Formatting Keywords: has-patch wptexturize 4.0-early
Focuses: Cc:

Description

I assume this is controversial, but I want wptexturize to *always* be a no-op. I know I'm not alone on this. There exists a 3rd-party "wpuntexturize" plugin, but it is insufficient; even using that plugin, I keep running in to places where texturization is happening anyway.

Rather than making us play whack-a-mole with all the places it gets turned back on, can't we please just have a checkbox to disable it globally?

Alternately, if you would make wptexturize be a pluggable function, I could just re-define it.

As it is, with each new release, I am forced to modify the source to add "return $text" as the first line of the function.

Attachments (3)

patch-wptexturize.patch (959 bytes) - added by toscho 2 years ago.
Add 'do_texturize' filter
19550.diff (1.1 KB) - added by nacin 2 years ago.
A run-once filter.
19550.2.diff (1003 bytes) - added by SergeyBiryukov 4 weeks ago.

Download all attachments as: .zip

Change History (35)

comment:1 scribu2 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Indeed, this has been brought up several times already: #17926 #17207

Please don't open new tickets for old issues. Instead, leave comments on the old tickets.

comment:2 toscho2 years ago

  • Cc info@… added
  • Resolution duplicate deleted
  • Status changed from closed to reopened

The request to make wptexturize() a pluggable function is not covered by these old tickets which address singular problems. I support this request, because wptexturize() is used in eight places directly, not per filter. So we should either move wptexturize() to pluggable.php or replace all direct calls with hooks.

comment:3 scribu2 years ago

  • Milestone set to Awaiting Review
  • Type changed from feature request to enhancement

Pluggable functions are a lousy way to fix this.

We should replace all direct calls with hooks.

comment:4 toscho2 years ago

List of direct calls to wptexturize()

wp-admin/includes/plugin.php:143
	$plugin_data['Description'] = wptexturize( $plugin_data['Description'] );
	
wp-admin/import.php:115
	$action = "<a href='" . esc_url("admin.php?import=$id") . "' title='" . esc_attr( wptexturize(strip_tags($data[1])) ) ."'>{$data[0]}</a>";

wp-admin/menu-header.php:75
	$title = wptexturize( $item[0] );

wp-admin/menu-header.php:148
	$title = wptexturize($sub_item[0]);

wp-includes/comment-template.php:758
	echo '    dc:title="'.str_replace('--', '&#x2d;&#x2d;', wptexturize(strip_tags(get_the_title()))).'"'."\n";

wp-includes/general-template.php:833
	$text = wptexturize($text);
	
wp-includes/media.php:867
	" . wptexturize($attachment->post_excerpt) . "

wp-includes/theme.php:217
	$theme_data['Description'] = wptexturize( wp_kses( $theme_data['Description'], $themes_allowed_tags ) );

wp-includes/theme.php:290
	$description = wptexturize($theme_data['Description']);

The question is how we name such a filter. And if we need more than one name.

comment:5 follow-up: scribu2 years ago

Actually, what would be the point in disabling wptexturize() for plugin and theme descriptions?

wp-admin/includes/plugin.php:143
	$plugin_data['Description'] = wptexturize( $plugin_data['Description'] );

wp-includes/theme.php:217
	$theme_data['Description'] = wptexturize( wp_kses( $theme_data['Description'], $themes_allowed_tags ) );

wp-includes/theme.php:290
	$description = wptexturize($theme_data['Description']);

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

Replying to scribu:

Actually, what would be the point in disabling wptexturize() for plugin and theme descriptions?

Okay, these cases aren’t important. :)

comment:7 scribu2 years ago

Relevant: #19602

comment:8 jwz2 years ago

The problem isn't just direct calls to wptexturize, it's things like this that occur in other plugins that are *also* trying to get around the effects of wptexturize, for example, Otto's Simple Facebook Connect does:

	remove_filter( 'the_content', 'wptexturize' );
	$text = apply_filters('the_content', $text);
	add_filter( 'the_content', 'wptexturize' );

If wptexturize was a pluggable function that I could turn into a no-op, it wouldn't matter to me that random plugins re-add it to the_content filter because they think I want it there.

comment:9 toscho2 years ago

Good point.

What if we extend just the function?

function wptexturize($text) {
	$do_texturize = apply_filters( 'do_texturize', TRUE, $text );
	if ( ! $do_texturize )
		return $text;

	global $wp_cockneyreplace;

Then you could disable wptexturize() case by case. More context for the filter would be better, of course …

comment:10 jwz2 years ago

Sure, that'd work for me...

toscho2 years ago

Add 'do_texturize' filter

comment:11 toscho2 years ago

To disable wptexturize() globally now use:

add_filter( 'do_texturize', '__return_false' );

comment:12 follow-up: scribu2 years ago

Looking again through this ticket, I still haven't heard an actual use-case for disabling wptexturize().

comment:13 toscho2 years ago

wptexturize() changes quotations, link titles, samp and var by default. Yes, you can filter each case, but if you don’t need the function at all? Plus, it is rather slow.

http://i.imgur.com/pEtYM.png

As @jwz explained: Sometimes you just cannot remove the function from a hook – a plugin may re-add it. So I see use cases here, and the additional hook doesn’t cost that much. :)

comment:14 follow-up: dd322 years ago

wptexturize() changes quotations, link titles, samp and var by default.

It sounds like your problems with the function might be things which you should focus on improving, rather than attempting to remove it entirely (which.. Isn't going to happen IMO)

comment:15 in reply to: ↑ 14 ; follow-up: toscho2 years ago

Replying to dd32:

It sounds like your problems with the function might be things which you should focus on improving, rather than attempting to remove it entirely (which.. Isn't going to happen IMO)

Of course, the details should be improved, and the function should not be removed. :)

The performance issues are caused by the regexes, I don’t see how to fix that.

The patch just gives more control for those who think they may need it. And it doesn’t hurt anyone.

comment:16 in reply to: ↑ 15 scribu2 years ago

Replying to toscho:

And it doesn’t hurt anyone.

Yes it does; each extra filter adds a tiny bit of overhead, both in terms of page load and of maintenance cost.

comment:17 toscho2 years ago

apply_filters() is called in this function anyway – so there are no measurable page load costs (do I miss anything?). Checking side effects is up to those who use the filter. The output of wptexturize() is already different for each language.

comment:18 in reply to: ↑ 12 jwz2 years ago

Replying to scribu:

Looking again through this ticket, I still haven't heard an actual use-case for disabling wptexturize().

If what you're asking is "why do I want to disable wptexturize?", I thought that would be obvious. That function does certain things -- namely taking the characters that I typed, and turning them into other characters -- and I don't want that. I want to see the characters I actually typed instead.

I'm not nitpicking just about how it's translating my quotes, or whether one particular hyphen is being rewritten right. I want it keep its hands completely off of my text. If I had wanted curly quotes then that's what I would have typed.

If other people want that, great. But please don't force this "feature" on me with no reasonable way to disable it.

nacin2 years ago

A run-once filter.

comment:19 follow-ups: nacin2 years ago

I'm wondering whether this is sufficient. While it will not catch *every* instance, it will catch all relevant core instances to dealing with content:

foreach ( array( 'the_title', 'the_content', 'the_excerpt',
		'comment_text', 'list_cats', 'comment_author',
		'term_name', 'link_name', 'link_description',
		'link_notes', 'bloginfo', 'wp_title', 'widget_title',
		'term_description'
	) as $filter )
		remove_filter( $filter, 'wptexturize' );

comment:20 in reply to: ↑ 19 toscho2 years ago

Replying to nacin:

I'm wondering whether this is sufficient. While it will not catch *every* instance, it will catch all relevant core instances to dealing with content:

Yeah, I have written a plugin like this some time ago. It doesn’t catch the problem @jwz mentioned and of course not the hard coded calls. The new hook would help to enforce a consistent behavior.

comment:21 in reply to: ↑ 19 nacin2 years ago

The code in comment 8 could be rewritten as such:

$priority = remove_filter( 'the_content', 'wptexturize' );
$text = apply_filters( 'the_content', $text );
if ( false !== $priority )
	add_filter( 'the_content', 'wptexturize', $priority );

comment:22 SergeyBiryukov2 years ago

  • Keywords has-patch added

comment:23 context6 months ago

  • Cc context added

comment:24 nacin3 months ago

  • Milestone changed from Awaiting Review to Future Release

It seems like 19550.diff would work.

comment:25 nacin3 months ago

  • Keywords wptexturize added

comment:26 follow-up: miqrogroove4 weeks ago

Testing the filter's result before running the filter seems unwise in 19550.diff.

comment:27 in reply to: ↑ 26 ; follow-up: SergeyBiryukov4 weeks ago

Replying to miqrogroove:

Testing the filter's result before running the filter seems unwise in 19550.diff.

The result in stored in a static variable for subsequent calls, I don't see anything wrong with that.

comment:28 in reply to: ↑ 27 miqrogroove4 weeks ago

Replying to SergeyBiryukov:

The result in stored in a static variable for subsequent calls, I don't see anything wrong with that.

If the purpose of the filter is to prevent wptexturize from altering the output, this patch will not do that. The filter result has to be tested after the filter, not before.

comment:29 miqrogroove4 weeks ago

See for yourself:

<?php
echo wptexturize( 'This is a test' );

function wptexturize($text) { 
    static $opening_quote, $run_texturize = true; 
    if ( false === $run_texturize )
        return $text; 
    if ( empty( $opening_quote ) ) { 
        $run_texturize = FALSE;  // apply_filters( 'run_wptexturize', $run_texturize ); 
        $opening_quote = '&#8220;';
    }
    $text = 'fail!';
    return $text;
}
?>
Last edited 4 weeks ago by miqrogroove (previous) (diff)

SergeyBiryukov4 weeks ago

comment:30 SergeyBiryukov4 weeks ago

I see what you mean, the filter only works for subsequent calls, not for the initial one.

19550.2.diff should work for every call, including the initial one.

comment:31 miqrogroove4 weeks ago

Now it looks like a good run-once filter.

I suggest 3.9 Milestone. Can't see any reason to keep this ticket open beyond that.

comment:32 SergeyBiryukov4 weeks ago

  • Keywords 4.0-early added
Note: See TracTickets for help on using tickets.