diff --git a/src/js/_enqueues/admin/comment.js b/src/js/_enqueues/admin/comment.js index 188caab2c30c5..b31f174bab5fe 100644 --- a/src/js/_enqueues/admin/comment.js +++ b/src/js/_enqueues/admin/comment.js @@ -99,4 +99,81 @@ jQuery( function($) { $edittimestamp.show().trigger( 'focus' ); $timestampdiv.slideUp( 'fast' ); }); + + var $commentparentdiv = $( '#comment-parent-div' ), + $commentparentdisplay = $( '#comment-parent-display' ), + originalcommentparentdisplay = $commentparentdisplay.html(), + $editcommentparent = $commentparentdiv.siblings( 'a.edit-comment-parent' ); + + /** + * Adds event that opens the parent comment form if the form is hidden. + * + * @since 7.1.0 + * + * @listens $editcommentparent:click + * + * @param {Event} event The event object. + * @return {void} + */ + $editcommentparent.on( 'click', function( event ) { + if ( $commentparentdiv.is( ':hidden' ) ) { + // Slide down the form and set focus on the parent field. + $commentparentdiv.slideDown( 'fast', function() { + $( '#comment_parent' ).trigger( 'focus' ); + } ); + $(this).hide(); + } + event.preventDefault(); + }); + + /** + * Resets the parent comment value when the cancel button is clicked. + * + * @since 7.1.0 + * + * @listens .cancel-comment-parent:click + * + * @param {Event} event The event object. + * @return {void} + */ + $commentparentdiv.find( '.cancel-comment-parent' ).on( 'click', function( event ) { + // Move focus back to the Edit link. + $editcommentparent.show().trigger( 'focus' ); + $commentparentdiv.slideUp( 'fast' ); + $( '#comment_parent' ).val( $( '#hidden_comment_parent' ).val() ); + $commentparentdisplay.html( originalcommentparentdisplay ); + event.preventDefault(); + }); + + /** + * Updates the parent comment display when the ok button is clicked. + * + * @since 7.1.0 + * + * @listens .save-comment-parent:click + * + * @param {Event} event The event object. + * @return {void} + */ + $commentparentdiv.find( '.save-comment-parent' ).on( 'click', function( event ) { + var $selected = $( '#comment_parent option:selected' ), + parentLabel = '0' === $selected.val() ? + /* translators: Displayed when a comment has no parent. */ + wp.i18n.__( 'None' ) : + $selected.data( 'author' ) || $selected.text(); + + event.preventDefault(); + + $commentparentdisplay.html( + wp.i18n.sprintf( + /* translators: %s: Parent comment link, or 'None'. */ + wp.i18n.__( 'In reply to: %s' ), + $( '' ).text( parentLabel ).prop( 'outerHTML' ) + ) + ); + + // Move focus back to the Edit link. + $editcommentparent.show().trigger( 'focus' ); + $commentparentdiv.slideUp( 'fast' ); + }); }); diff --git a/src/wp-admin/css/edit.css b/src/wp-admin/css/edit.css index 563151a3cdda6..e96cf4692fc03 100644 --- a/src/wp-admin/css/edit.css +++ b/src/wp-admin/css/edit.css @@ -554,6 +554,16 @@ form#tags-filter { line-height: 1.76923076; } +#comment-parent-div { + padding-top: 5px; + /* Fieldsets cannot shrink below their content by default, allow it so the select cannot cause overflow. */ + min-width: 0; +} + +#comment-parent-div select { + width: 100%; +} + #timestampdiv p { margin: 8px 0 6px; } diff --git a/src/wp-admin/edit-form-comment.php b/src/wp-admin/edit-form-comment.php index e0bbc9f657a73..935a26aee479a 100644 --- a/src/wp-admin/edit-form-comment.php +++ b/src/wp-admin/edit-form-comment.php @@ -201,25 +201,183 @@ comment_parent ) : +$parent_display = __( 'None' ); + +if ( $comment->comment_parent ) { $parent = get_comment( $comment->comment_parent ); - if ( $parent ) : - $parent_link = esc_url( get_comment_link( $parent ) ); - $name = get_comment_author( $parent ); - ?> -
comment_parent ) { + if ( ! get_option( 'thread_comments' ) ) { + return new WP_Error( 'comment_parent_invalid', __( 'The comment parent cannot be changed because threaded comments are disabled.' ) ); + } + + if ( $comment_parent === $comment_id ) { + return new WP_Error( 'comment_parent_invalid', __( 'A comment cannot be a reply to itself.' ) ); + } + + $parent = get_comment( $comment_parent ); + + // The parent must be a comment of the same type, on the same post, and not in the Trash or marked as spam. + if ( + ! $parent + || (int) $parent->comment_post_ID !== (int) $comment->comment_post_ID + || $parent->comment_type !== $comment->comment_type + || in_array( wp_get_comment_status( $parent ), array( 'spam', 'trash' ), true ) + ) { + return new WP_Error( 'comment_parent_invalid', __( 'Invalid parent comment.' ) ); + } + + // Walk up the new parent's ancestors to prevent creating a threading loop. + $ancestors = array(); + $ancestor = $parent; + $parent_depth = 1; + + while ( $ancestor && $ancestor->comment_parent && ! isset( $ancestors[ $ancestor->comment_ID ] ) ) { + if ( (int) $ancestor->comment_parent === $comment_id ) { + return new WP_Error( 'comment_parent_invalid', __( 'A comment cannot be a reply to one of its own replies.' ) ); + } + + $ancestors[ $ancestor->comment_ID ] = true; + ++$parent_depth; + + $ancestor = get_comment( $ancestor->comment_parent ); + } + + $max_thread_depth = (int) get_option( 'thread_comments_depth' ); + + if ( $max_thread_depth ) { + /* + * The comment's replies move with it, so the whole subtree must stay within + * the maximum depth. Measure its height one level of replies at a time, + * stopping as soon as the subtree cannot fit, which also bounds the loop + * should the stored comment hierarchy contain a cycle. + */ + $subtree_height = 1; + $level_ids = array( $comment_id ); + + while ( $level_ids && $parent_depth + $subtree_height <= $max_thread_depth ) { + $level_ids = get_comments( + array( + 'parent__in' => $level_ids, + 'fields' => 'ids', + 'status' => 'any', + 'orderby' => 'none', + ) + ); + + if ( $level_ids ) { + ++$subtree_height; + } + } + + if ( $parent_depth + $subtree_height > $max_thread_depth ) { + return new WP_Error( 'comment_parent_invalid', __( 'The comment cannot be moved there because it or its replies would exceed the maximum threading depth.' ) ); + } + } + } + } + foreach ( array( 'aa', 'mm', 'jj', 'hh', 'mn' ) as $timeunit ) { if ( ! empty( $_POST[ 'hidden_' . $timeunit ] ) && $_POST[ 'hidden_' . $timeunit ] !== $_POST[ $timeunit ] ) { $_POST['edit_date'] = '1'; diff --git a/tests/phpunit/tests/admin/EditFormComment_Test.php b/tests/phpunit/tests/admin/EditFormComment_Test.php new file mode 100644 index 0000000000000..f75785b0495e9 --- /dev/null +++ b/tests/phpunit/tests/admin/EditFormComment_Test.php @@ -0,0 +1,181 @@ +user->create( array( 'role' => 'administrator' ) ); + self::$post_id = $factory->post->create(); + } + + public function set_up() { + parent::set_up(); + + wp_set_current_user( self::$admin_id ); + } + + /** + * Renders the Edit Comment form for a comment and returns the output. + * + * @param int $comment_id Comment ID. + * @return string The rendered form. + */ + private function render_edit_form( $comment_id ) { + global $comment; + + $comment = get_comment_to_edit( $comment_id ); + $action = 'editcomment'; + + set_current_screen( 'comment' ); + + ob_start(); + require ABSPATH . 'wp-admin/edit-form-comment.php'; + $output = ob_get_clean(); + + unset( $GLOBALS['comment'] ); + + return $output; + } + + /** + * Returns the parent comment dropdown markup from the rendered form. + * + * @param int $comment_id Comment ID. + * @return string The dropdown markup, or an empty string if not rendered. + */ + private function get_parent_dropdown( $comment_id ) { + $output = $this->render_edit_form( $comment_id ); + + if ( ! preg_match( '||s', $output, $matches ) ) { + return ''; + } + + return $matches[0]; + } + + /** + * @ticket 65570 + */ + public function test_should_list_valid_parents_and_exclude_invalid_ones() { + $top_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $reply_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $top_id, + ) + ); + $spam_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_approved' => 'spam', + ) + ); + $trash_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_approved' => 'trash', + ) + ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $child_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $comment_id, + ) + ); + + $dropdown = $this->get_parent_dropdown( $comment_id ); + + $this->assertStringContainsString( 'value="0"', $dropdown, 'The "None" option should be listed.' ); + $this->assertStringContainsString( "value='{$top_id}'", $dropdown, 'A top-level comment should be listed.' ); + $this->assertStringContainsString( "value='{$reply_id}'", $dropdown, 'A nested comment should be listed.' ); + $this->assertStringNotContainsString( "value='{$comment_id}'", $dropdown, 'The comment being edited should not be listed.' ); + $this->assertStringNotContainsString( "value='{$child_id}'", $dropdown, 'A reply to the comment being edited should not be listed.' ); + $this->assertStringNotContainsString( "value='{$spam_id}'", $dropdown, 'A spam comment should not be listed.' ); + $this->assertStringNotContainsString( "value='{$trash_id}'", $dropdown, 'A trashed comment should not be listed.' ); + } + + /** + * @ticket 65570 + */ + public function test_should_not_render_parent_selector_when_comment_threading_is_disabled() { + update_option( 'thread_comments', 0 ); + + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $output = $this->render_edit_form( $comment_id ); + + $this->assertStringContainsString( 'id="comment-parent-display"', $output, 'The parent should still be displayed.' ); + $this->assertStringNotContainsString( 'name="comment_parent"', $output, 'The parent selector should not be rendered.' ); + } + + /** + * @ticket 65570 + */ + public function test_should_account_for_replies_when_excluding_deep_parents() { + update_option( 'thread_comments_depth', 3 ); + + $top_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $mid_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $top_id, + ) + ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $comment_id, + ) + ); + + $dropdown = $this->get_parent_dropdown( $comment_id ); + + $this->assertStringContainsString( "value='{$top_id}'", $dropdown, 'A parent leaving room for the reply should be listed.' ); + $this->assertStringNotContainsString( "value='{$mid_id}'", $dropdown, 'A parent whose depth plus the replies would exceed the maximum should not be listed.' ); + } + + /** + * @ticket 65570 + */ + public function test_should_list_current_parent_even_when_it_is_not_a_valid_option() { + $spam_parent_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_approved' => 'spam', + ) + ); + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $spam_parent_id, + ) + ); + + $dropdown = $this->get_parent_dropdown( $comment_id ); + + $this->assertStringContainsString( "value='{$spam_parent_id}'", $dropdown, 'The current parent should be listed.' ); + $this->assertStringContainsString( ' selected>', $dropdown, 'The current parent should be selected.' ); + } +} diff --git a/tests/phpunit/tests/admin/includes/comment/EditComment_Test.php b/tests/phpunit/tests/admin/includes/comment/EditComment_Test.php new file mode 100644 index 0000000000000..13ab871f85598 --- /dev/null +++ b/tests/phpunit/tests/admin/includes/comment/EditComment_Test.php @@ -0,0 +1,356 @@ +user->create( array( 'role' => 'administrator' ) ); + self::$post_id = $factory->post->create(); + self::$other_post_id = $factory->post->create(); + } + + public function set_up() { + parent::set_up(); + + wp_set_current_user( self::$admin_id ); + } + + public function tear_down() { + $_POST = array(); + + parent::tear_down(); + } + + /** + * Calls edit_comment() with a comment ID and a new parent, as submitted from the Edit Comment screen. + * + * @param int $comment_id Comment ID. + * @param int $comment_parent New parent comment ID. + * @return int|WP_Error The edit_comment() return value. + */ + private function update_comment_parent( $comment_id, $comment_parent ) { + $_POST = array( + 'comment_ID' => $comment_id, + 'comment_parent' => $comment_parent, + ); + + return edit_comment(); + } + + /** + * @ticket 65570 + */ + public function test_should_update_comment_parent() { + $parent_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $parent_id ); + + $this->assertSame( 1, $result ); + $this->assertSame( (string) $parent_id, get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_allow_clearing_comment_parent() { + $parent_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $parent_id, + ) + ); + + $result = $this->update_comment_parent( $comment_id, 0 ); + + $this->assertSame( 1, $result ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_parent_on_a_different_post() { + $parent_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$other_post_id ) ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $parent_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_nonexistent_parent() { + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $comment_id + 1000 ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_comment_as_its_own_parent() { + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $comment_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_child_as_parent() { + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $child_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $comment_id, + ) + ); + + $result = $this->update_comment_parent( $comment_id, $child_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_descendant_as_parent() { + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $child_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $comment_id, + ) + ); + $grandchild_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $child_id, + ) + ); + + $result = $this->update_comment_parent( $comment_id, $grandchild_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + } + + /** + * @ticket 65570 + */ + public function test_should_allow_resubmitting_unchanged_parent() { + $parent_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $parent_id, + ) + ); + + $result = $this->update_comment_parent( $comment_id, $parent_id ); + + $this->assertNotWPError( $result ); + $this->assertSame( (string) $parent_id, get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_new_parent_when_comment_threading_is_disabled() { + update_option( 'thread_comments', 0 ); + + $parent_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $parent_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_allow_clearing_parent_when_comment_threading_is_disabled() { + update_option( 'thread_comments', 0 ); + + $parent_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $comment_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $parent_id, + ) + ); + + $result = $this->update_comment_parent( $comment_id, 0 ); + + $this->assertSame( 1, $result ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_parent_at_maximum_threading_depth() { + update_option( 'thread_comments_depth', 2 ); + + $top_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $child_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $top_id, + ) + ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $child_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_parent_when_replies_would_exceed_maximum_threading_depth() { + update_option( 'thread_comments_depth', 3 ); + + $top_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $mid_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $top_id, + ) + ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $comment_id, + ) + ); + + // The comment itself would be at depth 3, but its reply would end up at depth 4. + $result = $this->update_comment_parent( $comment_id, $mid_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_allow_moving_comment_with_replies_within_maximum_threading_depth() { + update_option( 'thread_comments_depth', 3 ); + + $top_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_parent' => $comment_id, + ) + ); + + // The comment ends up at depth 2 and its reply at depth 3, the maximum. + $result = $this->update_comment_parent( $comment_id, $top_id ); + + $this->assertSame( 1, $result ); + $this->assertSame( (string) $top_id, get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + */ + public function test_should_reject_parent_of_a_different_comment_type() { + $parent_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_type' => 'pingback', + ) + ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $parent_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * @ticket 65570 + * + * @dataProvider data_should_reject_spam_or_trashed_parent + * + * @param string $comment_approved The parent's comment_approved value. + */ + public function test_should_reject_spam_or_trashed_parent( $comment_approved ) { + $parent_id = self::factory()->comment->create( + array( + 'comment_post_ID' => self::$post_id, + 'comment_approved' => $comment_approved, + ) + ); + $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); + + $result = $this->update_comment_parent( $comment_id, $parent_id ); + + $this->assertWPError( $result ); + $this->assertSame( 'comment_parent_invalid', $result->get_error_code() ); + $this->assertSame( '0', get_comment( $comment_id )->comment_parent ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_reject_spam_or_trashed_parent() { + return array( + 'a spam parent' => array( 'spam' ), + 'a trashed parent' => array( 'trash' ), + ); + } +}