[wp-trac] [WordPress Trac] #36901: Removing wp_die() from wp_allow_comment()

WordPress Trac noreply at wordpress.org
Sun May 22 16:33:38 UTC 2016


#36901: Removing wp_die() from wp_allow_comment()
----------------------------------------+-----------------------------
 Reporter:  websupporter                |       Owner:
     Type:  enhancement                 |      Status:  new
 Priority:  normal                      |   Milestone:  Future Release
Component:  Comments                    |     Version:  trunk
 Severity:  normal                      |  Resolution:
 Keywords:  has-unit-tests needs-patch  |     Focuses:
----------------------------------------+-----------------------------
Changes (by boonebgorges):

 * keywords:  has-patch dev-feedback needs-testing has-unit-tests => has-
     unit-tests needs-patch
 * milestone:  Awaiting Review => Future Release


Comment:

 +1 to the general idea of making these functions more testable and
 flexible.

 As noted in the Slack discussion linked above, [attachment:36901.patch]
 introduces a number of backward compatibility breaks that we should try to
 avoid. First, the current behavior of `check_comment()` - and thus of
 calling functions like `wp_allow_comment()` and `wp_new_comment()` - is to
 `die`. This default behavior probably cannot safely be changed, at least
 not without extensive research into who (outside of core) is using the
 function, and how.

 Additionally, we cannot change the function signature of
 `check_comment_flood_ids()`. This will break all existing uses of the
 function.

 A different strategy would be to introduce a new parameter to
 `wp_allow_comment()`, `check_comment()`, and related functions. This
 parameter would be something like `$return_error`, and would default to
 false. If `true`, it would return a `WP_Error` instead of `die`ing. We may
 then decide to refactor `wp_new_comment()` and friends so that `die`
 behavior is handled in wp-comments-post.php. But this could be a separate
 project from adding the new parameter, and the new tests that the param
 would enable, etc.

 A brief note that it's possible to work around some of these problems
 currently by registering a custom `wp_die_handler`.

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


More information about the wp-trac mailing list