Opened 3 years ago
Closed 2 years ago
#55664 closed defect (bug) (fixed)
WP post embed: missing labels and duplicate IDs
Reported by: | afercia | Owned by: | joedolson |
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Embeds | Keywords: | has-screenshots has-patch commit |
Focuses: | accessibility | Cc: |
Description (last modified by )
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, noaria-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.
Attachments (3)
Change History (21)
#4
@
3 years ago
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.
#5
@
3 years ago
- Milestone changed from Awaiting Review to Future Release
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.
#6
@
3 years ago
- Keywords needs-refresh added
- Milestone changed from Future Release to Awaiting Review
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.
#7
@
3 years ago
Random numbers have downsides, see #44883
Why does using the post ID for the IDs not work?
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
3 years ago
#11
@
3 years ago
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()
.
#12
@
3 years ago
Using the post-ID + wp_rand()
sounds like a reasonable and pretty performant solution 👍
#13
@
3 years ago
- Keywords needs-refresh removed
55664.2.diff updates the previous patch and uses post-ID + wp_rand()
to generate unique IDs.
#15
@
3 years ago
- Keywords commit added
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.
55664.diff is a first pass:
uniqid()
for the IDs. Open to better ways to generate unique IDs.aria-label
attributes to label the input field and the textarea.