Skip to content

A11y: dedupe ids colliding between advanced/quick style settings (aria_id_unique) #3368

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
Crabcyborg merged 1 commit into from
Sep 18, 2026
+77 −12
Merged

A11y: dedupe ids colliding between advanced/quick style settings (aria_id_unique) #3368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions classes/views/styles/_form-title.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
}
?>
<div class="frm5 frm_form_field">
<label
for="frm_fieldset_color"
<label
for="frm_title_color"
class="frm-style-item-heading"><?php esc_html_e( 'Color', 'formidable' ); ?></label>
</div>
<div class="frm7 frm_form_field">
Expand All @@ -14,7 +14,7 @@ class="frm-style-item-heading"><?php esc_html_e( 'Color', 'formidable' ); ?></la
$frm_style->get_field_name( 'title_color' ),
$style->post_content['title_color'],
array(
'id' => 'frm_fieldset_color',
'id' => 'frm_title_color',
'action_slug' => 'title_color',
)
);
Expand Down
18 changes: 9 additions & 9 deletions classes/views/styles/_quick-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ class="frm-style-item-heading"><?php esc_html_e( 'Button Text', 'formidable' );
<hr class="frm12"/>

<div class="frm5 frm_form_field">
<label
for="frm_field_margin"
<label
for="frm_style_qsettings_field_margin"
class="frm-style-item-heading"><?php esc_html_e( 'Vertical Spacing', 'formidable' ); ?></label>
</div>
<div class="frm7 frm_form_field">
Expand All @@ -128,7 +128,7 @@ class="frm-style-item-heading"><?php esc_html_e( 'Vertical Spacing', 'formidable
null,
$style->post_content['field_margin'],
array(
'id' => 'frm_field_margin',
'id' => 'frm_style_qsettings_field_margin',
'max_value' => 100,
'will_change' => array(
$frm_style->get_field_name( 'field_margin' ),
Expand All @@ -139,8 +139,8 @@ class="frm-style-item-heading"><?php esc_html_e( 'Vertical Spacing', 'formidable
</div>

<div class="frm5 frm_form_field">
<label
for="frm_field_pad"
<label
for="frm_style_qsettings_field_pad"
class="frm-style-item-heading"><?php esc_html_e( 'Input Field Padding', 'formidable' ); ?></label>
</div>
<div class="frm7 frm_form_field">
Expand All @@ -149,7 +149,7 @@ class="frm-style-item-heading"><?php esc_html_e( 'Input Field Padding', 'formida
null,
$style->post_content['field_pad'],
array(
'id' => 'frm_field_pad',
'id' => 'frm_style_qsettings_field_pad',
'max_value' => 100,
'will_change' => array(
$frm_style->get_field_name( 'field_pad' ),
Expand Down Expand Up @@ -206,8 +206,8 @@ class="frm-style-item-heading"><?php esc_html_e( 'Field Shape', 'formidable' );
</div>

<div class="frm5 frm_form_field frm_hidden" data-frm-element="field-shape-corner-radius">
<label
for="frm_border_radius"
<label
for="frm_style_qsettings_border_radius"
class="frm-style-item-heading"><?php esc_html_e( 'Corner Radius', 'formidable' ); ?></label>
</div>
<div class="frm7 frm_form_field frm_hidden frm-md-z-index" data-frm-element="field-shape-corner-radius">
Expand All @@ -216,7 +216,7 @@ class="frm-style-item-heading"><?php esc_html_e( 'Corner Radius', 'formidable' )
null,
$style->post_content['border_radius'],
array(
'id' => 'frm_border_radius',
'id' => 'frm_style_qsettings_border_radius',
'max_value' => 50,
'will_change' => array(
$frm_style->get_field_name( 'border_radius' ),
Expand Down
19 changes: 19 additions & 0 deletions tests/phpunit/base/FrmUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,4 +843,23 @@ protected function use_frm_role( $role ) {
break;
}
}

/**
* Assert that none of the given HTML element ids appears more than once in some
* rendered HTML (aria_id_unique — a duplicate id breaks any ARIA property that
* references it, since the reference can no longer resolve to a single element).
*
* @since x.x
*
* @param string $html
* @param array $ids
*
* @return void
*/
protected function assert_no_duplicate_element_ids( $html, $ids ) {
foreach ( $ids as $id ) {
$count = preg_match_all( '/\bid=["\']' . preg_quote( $id, '/' ) . '["\']/', $html );
$this->assertSame( 1, $count, 'Expected exactly one element with id "' . $id . '"' );
}
}
}
46 changes: 46 additions & 0 deletions tests/phpunit/styles/test_FrmStylesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,52 @@ private function get_custom_stylesheet() {
return $stylesheet_urls;
}

/**
* The styler edit page's "Quick Settings" panel and its "Advanced Settings"
* accordion sections both render into the DOM unconditionally (only one is
* shown at a time via CSS), so an id reused between a quick-settings control
* and its advanced-settings equivalent collides and breaks any ARIA property
* that references it (aria_id_unique).
*
* @covers FrmStylesController::render_style_page
*/
public function test_render_style_page_has_no_duplicate_ids() {
$this->set_current_user_to_1();

// render_style_page() reads $_GET to decide the view ('edit' vs 'list'); a leftover
// 'form'/'style_id' from another test would silently switch this to the list view.
$_GET = array();

$form_id = $this->factory->form->create();
$form = FrmForm::getOne( $form_id );
$frm_style = new FrmStyle( 'default' );
$active_style = $frm_style->get_one();

ob_start();
$this->run_private_method(
array( 'FrmStylesController', 'render_style_page' ),
array( $active_style, $form, $active_style )
);
$html = ob_get_clean();

$this->assert_no_duplicate_element_ids(
$html,
array(
'frm_field_pad',
'frm_field_margin',
'frm_border_radius',
'frm_fieldset_color',
// The renamed quick-settings/form-title ids themselves, so a future edit that
// deletes one of these elements (instead of just re-duplicating its id) still
// fails loudly here.
'frm_style_qsettings_field_pad',
'frm_style_qsettings_field_margin',
'frm_style_qsettings_border_radius',
'frm_title_color',
)
);
}

/**
* @covers FrmStylesController::save_style
* @covers FrmStyle::update
Expand Down
Loading