Skip to content

refactored: landmark link generation and add unit tests for edac_gene… #1085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SteveJonesDev merged 6 commits into from
Jul 21, 2025
Merged

refactored: landmark link generation and add unit tests for edac_gene… #1085

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e0bae1d
refactored: landmark link generation and add unit tests for edac_gene…
SteveJonesDev Jul 15, 2025
0523e82
updated: test escaped html output
SteveJonesDev Jul 15, 2025
5803137
refactored: update landmark selector assertion to decode before compa…
SteveJonesDev Jul 15, 2025
85eb701
refactored: enhance test for edac_generate_landmark_link to verify ba…
SteveJonesDev Jul 15, 2025
4dbf120
updated: comparing the decoded selector values instead of the encoded…
SteveJonesDev Jul 15, 2025
c88460e
refactored: update edac_generate_landmark_link to format landmark tex…
SteveJonesDev Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions admin/class-ajax.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,25 +484,10 @@ function ( $a, $b ) {

$html .= '<div class="edac-details-rule-records-record-cell edac-details-rule-records-record-landmark">';

$landmark = isset( $row['landmark'] ) ? esc_html( $row['landmark'] ) : '';
$landmark = isset( $row['landmark'] ) ? $row['landmark'] : '';
$landmark_selector = isset( $row['landmark_selector'] ) ? $row['landmark_selector'] : '';

if ( $landmark && $landmark_selector ) {
$landmark_url = add_query_arg(
[
'edac_landmark' => base64_encode( $landmark_selector ),
'edac_nonce' => wp_create_nonce( 'edac_highlight' ),
],
get_the_permalink( $postid )
);

// translators: %s is the landmark type (e.g., "Header", "Navigation", "Main").
$landmark_aria_label = sprintf( __( 'View %s landmark on website, opens a new window', 'accessibility-checker' ), ucwords( $landmark ) );
// translators: %s is the landmark type (e.g., "Header", "Navigation", "Main").
$html .= '<a href="' . $landmark_url . '" class="edac-details-rule-records-record-landmark-link" target="_blank" aria-label="' . esc_attr( $landmark_aria_label ) . '">' . ucwords( $landmark ) . '</a>';
} elseif ( $landmark ) {
$html .= ucwords( $landmark );
}
$html .= edac_generate_landmark_link( $landmark, $landmark_selector, $postid );

$html .= '</div>';

Expand Down
47 changes: 47 additions & 0 deletions includes/helper-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,3 +749,50 @@ function edac_remove_corrected_posts( $post_ID, $type, $pre = 1, $ruleset = 'php
)
);
}

/**
* Generate a landmark link with proper URL and ARIA label
*
* @param string $landmark The landmark type (e.g., "header", "navigation", "main").
* @param string $landmark_selector The CSS selector for the landmark.
* @param int $post_id The post ID to link to.
* @param string $css_class Optional CSS class for the link. Default 'edac-details-rule-records-record-landmark-link'.
* @param bool $target_blank Whether to open link in new window. Default true.
*
* @return string The HTML for the landmark link or just the landmark text if no selector.
*/
function edac_generate_landmark_link( $landmark, $landmark_selector, $post_id, $css_class = 'edac-details-rule-records-record-landmark-link', $target_blank = true ) {
$landmark = esc_html( $landmark );

@gemini-code-assist gemini-code-assist Bot Jul 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The esc_html() function is called before ucwords(). This can lead to issues if $landmark contains HTML entities, as ucwords() won't properly capitalize them after HTML escaping. It's better to capitalize first, then escape for output to prevent double-encoding or incorrect display of special characters. Consider capitalizing the landmark before escaping1.

function edac_generate_landmark_link( $landmark, $landmark_selector, $post_id, $css_class = 'edac-details-rule-records-record-landmark-link', $target_blank = true ) {
	if ( empty( $landmark ) ) {
		return '';
	}
	$landmark = ucwords( $landmark );
	$landmark = esc_html( $landmark );

Style Guide References

Footnotes

  1. Ensure data is properly formatted before escaping to prevent encoding issues. (link) ↩

@pattonwebz pattonwebz Jul 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it matters for our case because we shouldn't be passing in html entities directly but this comment is technically correct.

We could remove the early escape as it doesn't seem needed at all - or move it to wrap around the ucwords on lines 792 and 797.


if ( empty( $landmark ) ) {
return '';
}

// If we have both landmark and selector, create a link.
if ( ! empty( $landmark_selector ) ) {
$landmark_url = add_query_arg(
[
'edac_landmark' => base64_encode( $landmark_selector ),
'edac_nonce' => wp_create_nonce( 'edac_highlight' ),
],
get_the_permalink( $post_id )
);

// translators: %s is the landmark type (e.g., "Header", "Navigation", "Main").
$landmark_aria_label = sprintf( __( 'View %s landmark on website, opens a new window', 'accessibility-checker' ), ucwords( $landmark ) );

$target_attr = $target_blank ? ' target="_blank"' : '';

return sprintf(
'<a href="%s" class="%s"%s aria-label="%s">%s</a>',
esc_url( $landmark_url ),
esc_attr( $css_class ),
$target_attr,
esc_attr( $landmark_aria_label ),
ucwords( $landmark )
);
}

// If we only have landmark text, return it formatted.
return ucwords( $landmark );
}
1 change: 1 addition & 0 deletions src/frontendHighlighterApp/sass/app.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ body {
outline-offset: 5px !important;
outline-color: magenta !important;
max-width: calc(100vw - 30px) !important;
margin-left: 15px !important;
Comment thread
pattonwebz marked this conversation as resolved.
box-sizing: border-box !important;

&-min-width {
Expand Down
264 changes: 264 additions & 0 deletions tests/phpunit/helper-functions/GenerateLandmarkLinkTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
<?php
/**
* Class GenerateLandmarkLinkTest
*
* @package Accessibility_Checker
*/

/**
* Test case for edac_generate_landmark_link function.
*/
class GenerateLandmarkLinkTest extends WP_UnitTestCase {

/**
* Test post ID for testing.
*
* @var int
*/
private $test_post_id;

/**
* Set up test environment.
*/
public function setUp(): void {
parent::setUp();

// Create a test post.
$this->test_post_id = $this->factory->post->create(
[
'post_title' => 'Test Post',
'post_content' => 'Test content',
'post_status' => 'publish',
]
);
}

/**
* Clean up test environment.
*/
public function tearDown(): void {
// Clean up the test post.
wp_delete_post( $this->test_post_id, true );
parent::tearDown();
}

/**
* Tests the edac_generate_landmark_link function with valid landmark and selector.
*/
public function test_edac_generate_landmark_link_with_selector() {
$landmark = 'header';
$landmark_selector = 'header.site-header';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

// Check that result contains an anchor tag.
$this->assertStringContainsString( '<a href=', $result );
$this->assertStringContainsString( 'class="edac-details-rule-records-record-landmark-link"', $result );
$this->assertStringContainsString( 'target="_blank"', $result );
$this->assertStringContainsString( '>Header</a>', $result );

// Check that the URL contains the expected query parameters.
$this->assertStringContainsString( 'edac_landmark=', $result );
$this->assertStringContainsString( 'edac_nonce=', $result );

// Check aria-label.
$this->assertStringContainsString( 'aria-label="View Header landmark on website, opens a new window"', $result );

// Verify the landmark selector is base64 encoded in the URL.
$encoded_selector = base64_encode( $landmark_selector );
$this->assertStringContainsString( "edac_landmark={$encoded_selector}", $result );
}

/**
* Tests the edac_generate_landmark_link function with only landmark (no selector).
*/
public function test_edac_generate_landmark_link_without_selector() {
$landmark = 'navigation';
$landmark_selector = '';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

// Should return just the formatted landmark text, not a link.
$this->assertEquals( 'Navigation', $result );
$this->assertStringNotContainsString( '<a href=', $result );
$this->assertStringNotContainsString( 'edac_landmark=', $result );
}

/**
* Tests the edac_generate_landmark_link function with empty landmark.
*/
public function test_edac_generate_landmark_link_with_empty_landmark() {
$landmark = '';
$landmark_selector = 'nav.main-nav';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

// Should return empty string.
$this->assertEquals( '', $result );
}

/**
* Tests the edac_generate_landmark_link function with custom CSS class.
*/
public function test_edac_generate_landmark_link_with_custom_css_class() {
$landmark = 'main';
$landmark_selector = 'main.content';
$custom_class = 'my-custom-landmark-class';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id, $custom_class );

$this->assertStringContainsString( "class=\"{$custom_class}\"", $result );
$this->assertStringNotContainsString( 'class="edac-details-rule-records-record-landmark-link"', $result );
}

/**
* Tests the edac_generate_landmark_link function with target_blank disabled.
*/
public function test_edac_generate_landmark_link_without_target_blank() {
$landmark = 'footer';
$landmark_selector = 'footer.site-footer';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id, 'edac-details-rule-records-record-landmark-link', false );

$this->assertStringContainsString( '<a href=', $result );
$this->assertStringNotContainsString( 'target="_blank"', $result );
$this->assertStringContainsString( '>Footer</a>', $result );
}

/**
* Tests the edac_generate_landmark_link function with various landmark types.
*
* @dataProvider landmark_types_data_provider
*
* @param string $landmark The landmark type.
* @param string $expected_display The expected display text.
*/
public function test_edac_generate_landmark_link_landmark_types( $landmark, $expected_display ) {
$landmark_selector = 'div.test';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

$this->assertStringContainsString( ">{$expected_display}</a>", $result );
$this->assertStringContainsString( "View {$expected_display} landmark on website", $result );
}

/**
* Data provider for landmark types testing.
*
* @return array
*/
public function landmark_types_data_provider() {
return [
'header landmark' => [ 'header', 'Header' ],
'navigation landmark' => [ 'navigation', 'Navigation' ],
'main landmark' => [ 'main', 'Main' ],
'footer landmark' => [ 'footer', 'Footer' ],
'aside landmark' => [ 'aside', 'Aside' ],
'section landmark' => [ 'section', 'Section' ],
'search landmark' => [ 'search', 'Search' ],
'banner landmark' => [ 'banner', 'Banner' ],
'contentinfo landmark' => [ 'contentinfo', 'Contentinfo' ],
'complementary landmark' => [ 'complementary', 'Complementary' ],
'form landmark' => [ 'form', 'Form' ],
'region landmark' => [ 'region', 'Region' ],
];
}

/**
* Tests the edac_generate_landmark_link function with special characters in landmark.
*/
public function test_edac_generate_landmark_link_with_special_characters() {
$landmark = '<script>alert("xss")</script>';
$landmark_selector = 'div.test';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

// Should be properly escaped.
$this->assertStringNotContainsString( '<script>', $result );
$this->assertStringNotContainsString( 'alert("xss")', $result );
$this->assertStringContainsString( '&lt;Script&gt;Alert(&quot;Xss&quot;)&lt;/Script&gt;', $result );

@gemini-code-assist gemini-code-assist Bot Jul 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The assertion assertStringContainsString is used to check for the presence of a specific string within the result. However, after applying esc_html, the special characters in the landmark will be encoded. Therefore, the assertion should check for the encoded string &lt;Script&gt;Alert(&quot;Xss&quot;)&lt;/Script&gt; instead of the raw HTML.

$this->assertStringContainsString( '&lt;Script&gt;Alert(&quot;Xss&quot;)&lt;/Script&gt;', $result );

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

/**
* Tests the edac_generate_landmark_link function with special characters in selector.
*/
public function test_edac_generate_landmark_link_with_special_selector() {
$landmark = 'main';
$landmark_selector = 'div[data-test="value with spaces & symbols"]';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

// Should contain encoded selector.
$encoded_selector = base64_encode( $landmark_selector );
$this->assertStringContainsString( "edac_landmark={$encoded_selector}", $result );
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
$this->assertStringContainsString( '>Main</a>', $result );
}

/**
* Tests that the nonce is properly generated.
*/
public function test_edac_generate_landmark_link_nonce_generation() {
$landmark = 'header';
$landmark_selector = 'header.test';

// Generate the link twice.
$result1 = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );
$result2 = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

// Both should contain nonce parameters.
$this->assertStringContainsString( 'edac_nonce=', $result1 );
$this->assertStringContainsString( 'edac_nonce=', $result2 );

// Extract nonces (basic check that they're generated).
preg_match( '/edac_nonce=([^&"]+)/', $result1, $matches1 );
preg_match( '/edac_nonce=([^&"]+)/', $result2, $matches2 );

$this->assertNotEmpty( $matches1[1] );
$this->assertNotEmpty( $matches2[1] );

// Verify nonces are valid for the 'edac_highlight' action.
$this->assertTrue( wp_verify_nonce( $matches1[1], 'edac_highlight' ) !== false );
$this->assertTrue( wp_verify_nonce( $matches2[1], 'edac_highlight' ) !== false );
}

/**
* Tests the edac_generate_landmark_link function with null values.
*/
public function test_edac_generate_landmark_link_with_null_values() {
$result = edac_generate_landmark_link( null, null, $this->test_post_id );
$this->assertEquals( '', $result );

$result = edac_generate_landmark_link( 'main', null, $this->test_post_id );
$this->assertEquals( 'Main', $result );

$result = edac_generate_landmark_link( null, 'div.test', $this->test_post_id );
$this->assertEquals( '', $result );
}

/**
* Tests the edac_generate_landmark_link function URL structure.
*/
public function test_edac_generate_landmark_link_url_structure() {
$landmark = 'navigation';
$landmark_selector = 'nav.primary';

$result = edac_generate_landmark_link( $landmark, $landmark_selector, $this->test_post_id );

// Extract the href value.
preg_match( '/href="([^"]+)"/', $result, $matches );
$this->assertNotEmpty( $matches[1] );

$url = $matches[1];
$parsed_url = wp_parse_url( html_entity_decode( $url ) );

// Should contain the post permalink structure.
$this->assertNotEmpty( $parsed_url['query'] );

// Parse query string.
parse_str( $parsed_url['query'], $query_params );

$this->assertArrayHasKey( 'edac_landmark', $query_params );
$this->assertArrayHasKey( 'edac_nonce', $query_params );
$this->assertEquals( base64_encode( $landmark_selector ), $query_params['edac_landmark'] );
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
}
Loading