diff --git a/classes/helpers/FrmEntriesListHelper.php b/classes/helpers/FrmEntriesListHelper.php index adc63095d2..a9ff715440 100644 --- a/classes/helpers/FrmEntriesListHelper.php +++ b/classes/helpers/FrmEntriesListHelper.php @@ -414,13 +414,15 @@ public function single_row( $item, $style = '' ) { if ( $this->column_name === 'cb' ) { $r .= "$checkbox"; } else { - $val = in_array( $column_name, $hidden, true ) ? '' : $this->column_value( $item ); - $r .= ""; - // phpcs:ignore Universal.Operators.StrictComparisons - if ( $column_name == $action_col ) { + $is_action_col = $column_name == $action_col; + $val = in_array( $column_name, $hidden, true ) ? '' : $this->column_value( $item, $is_action_col ); + $r .= ""; + + if ( $is_action_col ) { $edit_link = admin_url( 'admin.php?page=formidable-entries&frm_action=edit&id=' . $item->id ); - $r .= '' . $val . ' '; + $link_text = self::get_row_title_text( $val, $item ); + $r .= '' . $link_text . ' '; $r .= $action_links; } else { $r .= $val; @@ -433,6 +435,29 @@ public function single_row( $item, $style = '' ) { return $r . ''; } + /** + * Get the content of the row title link, so the link is never empty for screen readers. + * + * A link nested inside the row title link (like a field or custom column value with its own link) is invalid markup, + * and browsers close the row title link before it, leaving the row title link with no text. + * + * @since x.x + * + * @param mixed $val The column value. + * @param stdClass $item The entry. + * + * @return string + */ + private static function get_row_title_text( $val, $item ) { + $val = preg_replace( '/<\/?a\b[^>]*>/i', '', (string) $val ); + + if ( '' === trim( wp_strip_all_tags( $val ) ) ) { + $val .= '' . esc_html( self::get_entry_label( $item ) ) . ''; + } + + return $val; + } + /** * Get the column names that the logged in user can action on * @@ -444,10 +469,12 @@ private function get_action_columns() { /** * @param object $item + * @param bool $is_action_col Whether this column is the row's primary/action column + * (wrapped in the row-title link by the caller). * * @return mixed */ - private function column_value( $item ) { + private function column_value( $item, $is_action_col = false ) { $col_name = $this->maybe_fix_column_name( $this->column_name ); switch ( $col_name ) { @@ -484,14 +511,19 @@ private function column_value( $item ) { $form_id = $item->form_id; $user_can_edit_forms = false === FrmAppHelper::permission_nonce_error( 'frm_edit_forms' ); - if ( $user_can_edit_forms ) { + if ( $user_can_edit_forms && ! $is_action_col ) { + // When this column is the row's own action column, the caller already + // wraps it in a row-title link to the entry -- nesting a second here + // (to the form itself) produces invalid, empty-named markup once the + // browser closes the outer anchor early. Fall back to the plain label, + // same as when the user can't edit forms. $val = FrmFormsHelper::edit_form_link( $form_id ); } else { $val = FrmFormsHelper::edit_form_link_label( $form_id ); } break; case 'post_id': - $val = FrmAppHelper::post_edit_link( $item->post_id ); + $val = $this->get_post_column_value( $item, $is_action_col ); break; case 'user_id': $user = get_userdata( $item->user_id ); @@ -501,6 +533,8 @@ private function column_value( $item ) { $val = $item->parent_item_id; break; default: + // $is_action_col isn't passed to these filters. If a custom column value with its own + // becomes the row's action column, single_row() strips the nested link in get_row_title_text(). $val = apply_filters( 'frm_entries_' . $col_name . '_column', false, compact( 'item' ) ); if ( $val === false ) { @@ -521,6 +555,28 @@ private function column_value( $item ) { return $val; } + /** + * Get the Post column value. + * + * @since x.x + * + * @param object $item The entry. + * @param bool $is_action_col Whether this column is the row's primary/action column. + * + * @return string + */ + private function get_post_column_value( $item, $is_action_col ) { + if ( ! $is_action_col ) { + return FrmAppHelper::post_edit_link( $item->post_id ); + } + + // Same reasoning as the form_id case in column_value(): post_edit_link() builds its + // own nested , which breaks the outer row-title link when this column + // is the action column. + $post = get_post( $item->post_id ); + return $post ? FrmAppHelper::truncate( $post->post_title, 50 ) : FrmFormsHelper::get_no_title_text(); + } + /** * When a form has entries with the 0 item meta value, the values do not appear properly in the entries list. * diff --git a/tests/phpunit/entries/test_FrmEntriesListHelper.php b/tests/phpunit/entries/test_FrmEntriesListHelper.php index 2e786d840d..dbe07388c7 100644 --- a/tests/phpunit/entries/test_FrmEntriesListHelper.php +++ b/tests/phpunit/entries/test_FrmEntriesListHelper.php @@ -44,6 +44,96 @@ public function test_column_value() { $this->assertSame( 'My entry name', $column_value ); } + /** + * The Form column's own value is wrapped in a second, nested row-title + * link when it's the row's action column (e.g. Entry Key/ID columns + * hidden on the "all forms" Entries screen) -- browsers implicitly close + * the outer row-title as soon as they hit that nested one, leaving it + * with no accessible name. When acting as the action column, the form + * value must be plain text instead. + * + * @covers FrmEntriesListHelper::column_value + */ + public function test_column_value_form_id_as_action_col_has_no_nested_link() { + FrmAppHelper::set_current_screen_and_hook_suffix(); + wp_set_current_user( 1 ); + + $item = new stdClass(); + $item->form_id = $this->factory->form->create( array( 'name' => 'My Form' ) ); + + // Not the action column: existing behavior, links to the form itself. + $column_value = $this->column_value( $item, 'form_id', false ); + $this->assertStringContainsString( 'assertStringContainsString( 'My Form', $column_value ); + + // As the action column: no nested , plain label only. + $column_value = $this->column_value( $item, 'form_id', true ); + $this->assertStringNotContainsString( 'assertSame( 'My Form', $column_value ); + } + + /** + * An entry whose form no longer exists (form_id doesn't resolve to a + * row) must still render non-empty fallback text as the action column, + * not an empty row-title link. + * + * @covers FrmEntriesListHelper::column_value + */ + public function test_column_value_form_id_as_action_col_missing_form_falls_back() { + FrmAppHelper::set_current_screen_and_hook_suffix(); + wp_set_current_user( 1 ); + + $item = new stdClass(); + $item->form_id = 999999999; + + $column_value = $this->column_value( $item, 'form_id', true ); + $this->assertNotEmpty( $column_value ); + $this->assertStringNotContainsString( ', which must be dropped in favor of a plain label when this + * column is the row's action column. + * + * @covers FrmEntriesListHelper::column_value + */ + public function test_column_value_post_id_as_action_col_has_no_nested_link() { + FrmAppHelper::set_current_screen_and_hook_suffix(); + + $item = new stdClass(); + $item->post_id = $this->factory->post->create( array( 'post_title' => 'My Post' ) ); + + // Not the action column: existing behavior, links to the post itself. + $column_value = $this->column_value( $item, 'post_id', false ); + $this->assertStringContainsString( 'assertStringContainsString( 'My Post', $column_value ); + + // As the action column: no nested , plain label only. + $column_value = $this->column_value( $item, 'post_id', true ); + $this->assertStringNotContainsString( 'assertSame( 'My Post', $column_value ); + } + + /** + * An entry whose linked post no longer exists must still render + * non-empty fallback text as the action column, not an empty + * row-title link. + * + * @covers FrmEntriesListHelper::column_value + */ + public function test_column_value_post_id_as_action_col_missing_post_falls_back() { + FrmAppHelper::set_current_screen_and_hook_suffix(); + + $item = new stdClass(); + $item->post_id = 999999999; + + $column_value = $this->column_value( $item, 'post_id', true ); + $this->assertNotEmpty( $column_value ); + $this->assertStringNotContainsString( ' 5, + 'name' => '', + 'ip' => '', + 'form_id' => $this->factory->form->create( array( 'name' => 'My Form' ) ), + ); + + $row = $this->single_row( $item, array( '0_form_id' => 'Form' ) ); + $this->assertMatchesRegularExpression( '/]*class="row-title" >My Form<\/a>/', $row ); + $this->assertSame( 1, substr_count( $row, '>My Form<' ) ); + + // A custom column value with its own link is reduced to its text. + $custom_column = static function () { + return 'Custom value'; + }; + add_filter( 'frm_entries_custom_column', $custom_column ); + $row = $this->single_row( $item, array( '0_custom' => 'Custom' ) ); + remove_filter( 'frm_entries_custom_column', $custom_column ); + $this->assertStringContainsString( 'class="row-title" >Custom value', $row ); + $this->assertStringNotContainsString( 'example.com', $row ); + + // An empty value still gives the link an accessible name. + $row = $this->single_row( $item, array( '0_ip' => 'IP' ) ); + $this->assertStringContainsString( 'class="row-title" >Entry 5', $row ); + } + + /** + * @param stdClass $item + * @param array $columns + * + * @return string + */ + private function single_row( $item, $columns ) { + $list_helper = new FrmEntriesListHelper( array( 'params' => array( 'form' => 0 ) ) ); + $this->set_private_property( $list_helper, '_column_headers', array( $columns, array(), array(), key( $columns ) ) ); + return $list_helper->single_row( $item ); + } + /** * @param stdClass $item * @param string $column_name + * @param bool $is_action_col */ - private function column_value( $item, $column_name ) { + private function column_value( $item, $column_name, $is_action_col = false ) { $list_helper = new FrmEntriesListHelper( array() ); $this->set_private_property( $list_helper, 'column_name', $column_name ); - return $this->run_private_method( array( $list_helper, 'column_value' ), array( $item ) ); + return $this->run_private_method( array( $list_helper, 'column_value' ), array( $item, $is_action_col ) ); } }