WP post embed: missing labels and duplicate IDs

[afercia]

The form fields in the sharing dialog of the WordPress post embed aren't labeled. Also, when there's more than one WordPress post embed on a page, there are duplicate IDs.

To reproduce:

• Add a WordPress post embed twice. Add a third embed to another post.
• Click the 'share' icon on both the embeds to reveal the Sharing dialog.
• Check the URL input field and the HTML textarea aren't labeled: there's no associated <label> element, no aria-label or any other labeling.
• See the 'WordPress Embed' and 'HTML Embed' tab panels use the same IDs on both embeds.
• See the input field and textarea descriptions use the same IDs on both embeds.

These IDs are then used for some aria-* attributes e.g. aria-controls, aria-describedby thus referencing different elements with the same IDs.

See attached screenshot for reference.

[afercia]

55664.diff​ is a first pass:

• Uses uniqid() for the IDs. Open to better ways to generate unique IDs.
• Adds aria-label attributes to label the input field and the textarea.

[ocean90]

Replying to afercia:

• Uses uniqid() for the IDs. Open to better ways to generate unique IDs.

Sounds like a case where wp_unique_id() should be used instead, see #44883 for some background.

[swissspidy]

+1 to using wp_unique_id()

[afercia]

I did try wp_unique_id() as first thing but it always prints out 1 to me :) I think it's because the particular way print_embed_sharing_dialog is used but I didn't dive deep.

Edit: I guess the wp_unique_id() counter variable is actually processed in the php that's used within the iframe so... it always restarts the count for each iframe.

[swissspidy]

Ah right, that makes sense. Hmm.

What about using the post ID then instead of uniqid/wp_unique_id?

Seems unlikely that one would embed the same post twice on a page, or that two embeds from different websites have the same post IDs.

To address these edge cases, the IDs could be updated using JavaScript. For example by using the unique secret/hash passed to each iframe.

[afercia]

Thanks for the insight @swissspidy. I'd rather keep it simple. After all, the worst (very unlikely) thing that can happen is a missing association between labels and input fields.

To me, something like 'some-prefix' . wp_rand() would be enough. Thoughts?

I see the iframe unique secret is generated with wp_generate_password( 10, false ) but that seems a bit overkill to me. Also, I'd rather avoid using JavaScript. Other approaches used in core, see combinations of various functions e.g. md5( 'some-string' . time() . wp_rand() ) seem definitely overkill.

Please, let's keep the milestone to 'awaiting review' for now, so the accessibility team can review this ticket during their next bug scrub, especially for part related to the missing labels.

[swissspidy]

Random numbers have downsides, see #44883

Why does using the post ID for the IDs not work?

[afercia]

Why does using the post ID for the IDs not work?

Seems unlikely that one would embed the same post twice on a page, or that two embeds from different websites have the same post IDs.

It's unlikely but still possible. I'd propose a combination of the post ID + wp_rand().

[aristath]

Using the post-ID + wp_rand() sounds like a reasonable and pretty performant solution 👍

[afercia]

55664.2.diff​ updates the previous patch and uses post-ID + wp_rand() to generate unique IDs.

[joedolson]

Tested patch using NVDA & Jaws, aria-label values read as expected; no unlabeled fields. The changes are clear, and nothing is complicated. Marking for commit.

[joedolson]

In 53912:

Embeds: Fix missing labels and duplicate IDs.

Add aria-label and aria-describedby to provide embed form fields with accessible names and ensure each embed has a unique ID attribute to prevent duplicate IDs.

Props afercia, swissspidy.
Fixes #55664.