AphrontFormPolicyControl: Reuse existing control ID
ClosedPublic
Actions

Authored by aklapper on Mar 21 2025, 12:20.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T16019: AphrontFormPolicyControl label vs control ID are off by one

Commits

rPad7560b36491: AphrontFormPolicyControl: Reuse existing control ID

Summary

Do not unconditionally create a new ID which breaks the accessibility label.

Parent AphrontFormControl::render() set an ID via $this->setID(celerity_generate_unique_node_id()) and then calls $this->renderInput() on its subclass AphrontFormPolicyControl which unconditionally runs $control_id = celerity_generate_unique_node_id() to assign another ID and then does 'id' => $control_id.
This breaks accessibility (AphrontFormControl's label for="UQ0_xx" is now off by one from AphrontFormPolicyControl's a id="UQ0_xx").

Similar existing code can be found in AphrontFormTokenizerControl::renderInput() at https://we.phorge.it/source/phorge/browse/master/src/view/form/control/AphrontFormTokenizerControl.php;bccd4f5981a7e2c347e8e26cf24d8394c882724f$57-61

Closes T16019

Test Plan

Create a Maniphest Task form which exposes policy controls. Check the resulting HTML if <label for=""> and <a id=""> match.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Mar 21 2025, 12:20

Owners added a reviewer: O1: Blessed Committers.Mar 21 2025, 12:20

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptMar 21 2025, 12:20

aklapper requested review of this revision.Mar 21 2025, 12:20

Harbormaster completed remote builds in B1785: Diff 2750.Mar 21 2025, 12:20

Uhm yes thanks

There is probably a sub-bug about the inability of dropdown and tokenizers to be focused by label click, but it's unrelated to this I think. I've played with $input_id and it seems unrelated

This revision is now accepted and ready to land.Mar 22 2025, 22:33

Closed by commit rPad7560b36491: AphrontFormPolicyControl: Reuse existing control ID. · Explain WhyMar 23 2025, 09:12

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rPad7560b36491: AphrontFormPolicyControl: Reuse existing control ID.