[wp-trac] [WordPress Trac] #36666: Enhance `remove_theme_support()` so that it can take additional arguments

[WordPress Trac]

#36666: Enhance `remove_theme_support()` so that it can take additional arguments
------------------------------------+------------------------------
 Reporter:  flixos90                |       Owner:
     Type:  enhancement             |      Status:  new
 Priority:  normal                  |   Milestone:  Awaiting Review
Component:  Themes                  |     Version:  3.0
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |     Focuses:
------------------------------------+------------------------------

Comment (by jmichaelward):

 Replying to [comment:9 flixos90]:
 > Unfortunately we cannot use the `get_post_types_by_support()` function
 since it is targeted add the individual post type support for features
 like `title`, `editor`, `comments`, `thumbnails` and so on - it doesn't
 have to do with theme support. And at the point where most people would
 call `remove_theme_support()`, the post types will not have been
 registered yet, so we need to figure out another way to handle this. I
 think it's simply not possible to remove a specific post type from support
 if its current value is just a `true`.

 Perhaps I need to clarify: in patch
 [https://core.trac.wordpress.org/attachment/ticket/36666/36666.2.diff
 36666.2.diff], `get_post_types_by_support()` is only being called if the
 value for the `$_wp_theme_features['post-thumbnails']` is boolean (and,
 given earlier checks, specifically, that boolean is true). If a post type
 is registered with theme support for post thumbnails, and
 `add_theme_support( 'post-thumbnails' )` was called somewhere in a plugin
 or theme, then all post types that support post thumbnails will have them.
 I think this is a correct use case for this function.

 I agree that this cannot be used to incrementally de-register the other
 types of `theme_support`, but it should be appropriate for post
 thumbnails, since it simply indicates which post types support them. In
 theory, it ''could'' be used for `post-formats`, too, but to my knowledge,
 WordPress doesn't yet have a built-in mechanism for returning a list of
 registered post formats (because, as you said, `get_theme_support( 'post-
 formats' )` also returns true. Perhaps this is cause for a new ticket, as
 well?

 Consider this code block:

 {{{
 <?php
 <?php
 add_action( 'after_setup_theme', function() {
         // outputs true
         var_dump( get_theme_support( 'post-thumbnails' ) );

         /* Outputs an array of post types that support post thumbnails,
 which should be all of them.
          * In this case, WooCommerce is installed and has registered post
 thumbnails for all types. It also contains
          * a custom post type named `product`, which is included in this
 array.
          */
         var_dump( get_post_types_by_support( 'thumbnail' ) );

         // Let's remove post-thumbnails support for the product post type
         remove_theme_support( 'post-thumbnails', array( 'product' ) );

         // Outputs an updated array that excludes the product post type
         var_dump( get_theme_support( 'post-thumbnails' ) );
 });
 }}}

 WooCommerce is installed and activated, and it registers a post type
 called `product` and calls `add_theme_support( 'post-thumbnails' )` if the
 current theme does not support it. With patch 36666.2.diff applied, the
 previous block outputs the following:

 {{{
 boolean true
 array (size=5)
   0 => string 'post' (length=4)
   1 => string 'page' (length=4)
   2 => string 'attachment:audio' (length=16)
   3 => string 'attachment:video' (length=16)
   4 => string 'product' (length=7)
 array (size=1)
   0 =>
     array (size=4)
       0 => string 'post' (length=4)
       1 => string 'page' (length=4)
       2 => string 'attachment:audio' (length=16)
       3 => string 'attachment:video' (length=16)
 }}}

 In terms of the timing, I agree that there are issues. If
 `remove_theme_support()` is called in `functions.php` without a hook in
 the above case, it is impossible to remove `product` from the collection
 because it gets added during `after_setup_theme`. If, however, the
 function is called to remove the added post type during the same hook in
 which is was added, it can be removed. Consider the following code block:

 {{{
 <?php
 // Outputs null
 var_dump( get_theme_support( 'post-thumbnails' ) );

 add_theme_support( 'post-thumbnails' );

 // Outputs true
 var_dump( get_theme_support( 'post-thumbnails' ) );

 remove_theme_support( 'post-thumbnails', array( 'product' ) );

 // Outputs an array of post types, but doesn't include 'product'
 var_dump( get_theme_support( 'post-thumbnails' ) );

 /**
  * Run remove_theme_support() after theme is set up
  */
 add_action( 'after_setup_theme', function() {
         // Outputs an array of post types, including product
         var_dump( get_theme_support( 'post-thumbnails' ) );

         remove_theme_support( 'post-thumbnails', array( 'product' ) );

         var_dump( get_theme_support( 'post-thumbnails' ) );
 });
 }}}

 And its subsequent output:

 {{{
 boolean false
 boolean true
 array (size=1)
   0 =>
     array (size=4)
       0 => string 'post' (length=4)
       1 => string 'page' (length=4)
       2 => string 'attachment:audio' (length=16)
       3 => string 'attachment:video' (length=16)
 array (size=1)
   0 =>
     array (size=5)
       0 => string 'post' (length=4)
       1 => string 'page' (length=4)
       2 => string 'attachment:audio' (length=16)
       3 => string 'attachment:video' (length=16)
       4 => string 'product' (length=7)
 array (size=1)
   0 =>
     array (size=4)
       0 => string 'post' (length=4)
       1 => string 'page' (length=4)
       2 => string 'attachment:audio' (length=16)
       3 => string 'attachment:video' (length=16)
 }}}

 Do note that WooCommerce's current registration of post thumbnail support
 is problematic, but can be simplified from its current registration to
 work with this update:

 Current:
 {{{
         /**
          * Ensure post thumbnail support is turned on.
          */
         private function add_thumbnail_support() {
                 if ( ! current_theme_supports( 'post-thumbnails' ) ) {
                         add_theme_support( 'post-thumbnails' );
                 }
                 add_post_type_support( 'product', 'thumbnail' );
         }
 }}}

 Possible update:
 {{{
         /**
          * Ensure post thumbnail support is turned on.
          */
         private function add_thumbnail_support() {
                 add_theme_support( 'post-thumbnails' );
                 add_post_type_support( 'product', 'thumbnail' );
         }
 }}}

 This would return the output demonstrated in the previous set of blocks.

 I think this demonstrates that `get_post_types_by_support` is beneficial
 for this particular use case. Furthermore, I think it's advantageous to
 the user compared with patch 36666.3.diff, because users are not, in any
 case, able to remove post type support for an individual post type in this
 scenario with that patch. They call `remove_theme_support( 'post-
 thumbnails', array( 'post' )`, and nothing happens because `false` is
 returned.

 I propose we continue to explore this solution further, and see whether it
 can be applied to other types of theme support, including 'html5' and
 'post-formats'.

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