[wp-trac] [WordPress Trac] #29717: wp_check_invalid_utf8 - pcre tricks and failsafes, +mb_convert_encoding, iconv fix, performance

[WordPress Trac]

#29717: wp_check_invalid_utf8 - pcre tricks and failsafes, +mb_convert_encoding,
iconv fix, performance
--------------------------------+------------------------------------------
 Reporter:  askapache           |       Owner:
     Type:  enhancement         |      Status:  new
 Priority:  normal              |   Milestone:  Awaiting Review
Component:  Formatting          |     Version:  trunk
 Severity:  normal              |  Resolution:
 Keywords:  has-patch dev-      |     Focuses:  administration, performance
  feedback                      |
--------------------------------+------------------------------------------

Comment (by kitchin):

 Cool stuff. Comments:

 (1) I still think the old blog_charset check is clearest. No need to
 confuse people into having to look up obscure docs. Old code:
 {{{
 in_array( get_option( 'blog_charset' ), array( 'utf8', 'utf-8', 'UTF8',
 'UTF-8' ) )
 }}}
 vs. your new code
 {{{
 stripos( $is_utf8, 'utf' ) !== false && strpos( $is_utf8, '8' ) !== false
 }}}

 (2) The WP code base never checks the result of ini_set() or @ini_set()
 but in this case it seems wise to do so. Hosts can disallow it. Most
 robust way is probably:
 {{{
 static $mb_convert;
 if ( function_exists( 'mb_convert_encoding' ) ) {
   @ini_set( 'mbstring.substitute_character', 'none' );
   $mb_convert = @ini_get( 'mbstring.substitute_character' ) === 'none';
 }
 }}}

 I don't imagine anybody is worried about changing that ini value with
 restoring it, but it should probably be noted in the inline doc as a side
 effect.

 As for WP coding standards nits, WP wants braces on all clauses (if ...
 {}). Also, no parentheses around function_exists() at line 775.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/29717#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform