diff --git a/accessibility-checker.php b/accessibility-checker.php index 4211547a8..d1c7c8c90 100755 --- a/accessibility-checker.php +++ b/accessibility-checker.php @@ -41,7 +41,7 @@ // Current database version. if ( ! defined( 'EDAC_DB_VERSION' ) ) { - define( 'EDAC_DB_VERSION', '1.0.4' ); + define( 'EDAC_DB_VERSION', '1.0.5' ); } // Plugin Folder Path. diff --git a/admin/class-insert-rule-data.php b/admin/class-insert-rule-data.php index bccc97703..31a42d008 100644 --- a/admin/class-insert-rule-data.php +++ b/admin/class-insert-rule-data.php @@ -12,6 +12,14 @@ /** * Class for inserting rule data into the database * + * The unique identifier for issues changed in version 1.0.5 of the database schema. + * Previously, issues were identified by: postid + rule + object + type + siteid + * Now, issues are identified by: postid + rule + selector + type + siteid + * + * This change allows duplicate code objects (e.g., two empty paragraphs) to be + * stored as separate issues when they appear in different locations on the page. + * The selector field provides the unique location identifier for each issue. + * * @since 1.10.0 */ class Insert_Rule_Data { @@ -73,15 +81,17 @@ public function insert( object $post, string $rule, string $ruletype, string $ru } // Check if exists. + // Use selector as the unique identifier instead of object to allow duplicate code objects + // with different selectors (e.g., two empty paragraphs in different locations). // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Using direct query for adding data to database, caching not required for one time operation. $results = $wpdb->get_results( $wpdb->prepare( - 'SELECT postid, ignre FROM %i where type = %s and postid = %d and rule = %s and object = %s and siteid = %d', + 'SELECT postid, ignre FROM %i where type = %s and postid = %d and rule = %s and selector = %s and siteid = %d', $table_name, $rule_data['type'], $rule_data['postid'], $rule_data['rule'], - $rule_data['object'], + $rule_data['selector'], $rule_data['siteid'] ), ARRAY_A @@ -97,22 +107,23 @@ public function insert( object $post, string $rule, string $ruletype, string $ru } // update existing record. + // Use selector for WHERE clause instead of object to match on unique identifier. // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Using direct query for adding data to database, caching not required for one time operation. $wpdb->query( $wpdb->prepare( - 'UPDATE %i SET recordcheck = %d, landmark = %s, landmark_selector = %s, selector = %s, ancestry = %s, xpath = %s, ignre = %d WHERE siteid = %d and postid = %d and rule = %s and object = %s and type = %s', + 'UPDATE %i SET recordcheck = %d, landmark = %s, landmark_selector = %s, object = %s, ancestry = %s, xpath = %s, ignre = %d WHERE siteid = %d and postid = %d and rule = %s and selector = %s and type = %s', $table_name, 1, $rule_data['landmark'], $rule_data['landmark_selector'], - $rule_data['selector'], + $rule_data['object'], $rule_data['ancestry'], $rule_data['xpath'], $rule_data['ignre'], $rule_data['siteid'], $rule_data['postid'], $rule_data['rule'], - $rule_data['object'], + $rule_data['selector'], $rule_data['type'] ) ); diff --git a/admin/class-update-database.php b/admin/class-update-database.php index 66d932816..7480e3d0d 100644 --- a/admin/class-update-database.php +++ b/admin/class-update-database.php @@ -74,9 +74,38 @@ public function edac_update_database() { require_once ABSPATH . 'wp-admin/includes/upgrade.php'; dbDelta( $sql ); + // Run migration for selector-based unique identifiers if upgrading from older versions. + if ( version_compare( $db_version, '1.0.5', '<' ) ) { + $this->migrate_to_selector_based_unique_id(); + } } // Update database version option. update_option( 'edac_db_version', sanitize_text_field( EDAC_DB_VERSION ) ); } + + /** + * Migrate existing records to use selector-based unique identifiers. + * + * This migration handles records that were created before the selector field + * was used as the unique identifier. Records with NULL selectors will have + * a fallback identifier generated based on their ID to ensure uniqueness. + * + * @since 1.0.5 + * @return void + */ + private function migrate_to_selector_based_unique_id() { + global $wpdb; + $table_name = $wpdb->prefix . 'accessibility_checker'; + + // Find records with NULL or empty selectors and update them with a fallback value. + // Using the record ID ensures each record has a unique selector for backward compatibility. + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- One-time migration query. + $wpdb->query( + $wpdb->prepare( + "UPDATE %i SET selector = CONCAT('legacy-id-', id) WHERE selector IS NULL OR selector = ''", + $table_name + ) + ); + } } diff --git a/includes/classes/class-rest-api.php b/includes/classes/class-rest-api.php index 72d812e1c..70316a225 100644 --- a/includes/classes/class-rest-api.php +++ b/includes/classes/class-rest-api.php @@ -264,15 +264,7 @@ public function clear_issues_for_post( $request ) { * * @return string */ - public function filter_js_validation_html( string $html, string $rule_id, array $violation ): string { - // Add the selector to the violation message as empty paragraphs are almost always - // duplicate html fragments. Adding the selector makes it unique, so it can be saved. - if ( 'empty_paragraph_tag' === $rule_id ) { - $html .= $violation['selector'][0] - ? '// {{ ' . $violation['selector'][0] . ' }}' - : ''; - } - + public function filter_js_validation_html( string $html, string $rule_id, array $violation ): string { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed, VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- the variable was used previously and will be used in future most likely. // Use just the opening and closing tag, prevents storing entire page as the affected code. if ( 'html-has-lang' === $rule_id || 'document-title' === $rule_id ) { $html = preg_replace( '/^.*().*(<\/html>).*$/s', '$1...$2', $html ); diff --git a/tests/phpunit/Admin/InsertRuleDataTest.php b/tests/phpunit/Admin/InsertRuleDataTest.php index 1ce42dd59..23fc6efd3 100644 --- a/tests/phpunit/Admin/InsertRuleDataTest.php +++ b/tests/phpunit/Admin/InsertRuleDataTest.php @@ -83,4 +83,51 @@ public function testRuleInserterReturnLogic() { $current_row_count = $wpdb->get_var( "SELECT COUNT(*) FROM $this->table_name" ); // phpcs:ignore WordPress.DB -- caching not required for one time operation. $this->assertEquals( $initial_row_count + 1, $current_row_count ); } + + /** + * Tests that duplicate objects with different selectors are stored separately. + * This test verifies the fix for the issue where duplicate code objects + * (like two empty paragraphs) were only flagged as one issue. + */ + public function testDuplicateObjectsWithDifferentSelectors() { + $post = $this->factory()->post->create_and_get(); + $rule = 'empty_paragraph_tag'; + $ruletype = 'warning'; + $rule_obj = '

'; + + global $wpdb; + + $rule_inserter = new Insert_Rule_Data(); + $initial_row_count = $wpdb->get_var( "SELECT COUNT(*) FROM $this->table_name" ); // phpcs:ignore WordPress.DB -- caching not required for one time operation. + + // Insert first empty paragraph with selector 1. + $selectors_1 = [ + 'selector' => [ 'div.content > p:nth-child(1)' ], + 'ancestry' => [ 'div.content', 'p' ], + 'xpath' => [ '/html/body/div/p[1]' ], + ]; + $result_1 = $rule_inserter->insert( $post, $rule, $ruletype, $rule_obj, null, null, $selectors_1 ); + $this->assertIsInt( $result_1 ); + + // Insert second empty paragraph with different selector - should be stored as separate issue. + $selectors_2 = [ + 'selector' => [ 'div.content > p:nth-child(5)' ], + 'ancestry' => [ 'div.content', 'p' ], + 'xpath' => [ '/html/body/div/p[5]' ], + ]; + $result_2 = $rule_inserter->insert( $post, $rule, $ruletype, $rule_obj, null, null, $selectors_2 ); + $this->assertIsInt( $result_2 ); + + // Verify two separate records were created. + $current_row_count = $wpdb->get_var( "SELECT COUNT(*) FROM $this->table_name" ); // phpcs:ignore WordPress.DB -- caching not required for one time operation. + $this->assertEquals( $initial_row_count + 2, $current_row_count ); + + // Verify inserting the same object with same selector is treated as duplicate. + $result_3 = $rule_inserter->insert( $post, $rule, $ruletype, $rule_obj, null, null, $selectors_1 ); + $this->assertEquals( null, $result_3 ); + + // Verify row count hasn't changed. + $final_row_count = $wpdb->get_var( "SELECT COUNT(*) FROM $this->table_name" ); // phpcs:ignore WordPress.DB -- caching not required for one time operation. + $this->assertEquals( $initial_row_count + 2, $final_row_count ); + } }