Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#44966 closed defect (bug) (fixed)

Safari + Voiceover does not allow tags entry in editor

Reported by: parbaugh's profile parbaugh Owned by: afercia's profile afercia
Milestone: 5.1 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch commit
Focuses: accessibility Cc:

Description

This appears to be a bug limited to Safari and Voiceover, but it impacts the experience of users who rely on these technologies. We’ve been able to reproduce this across versions of MacOS, up to at least 10.13.6, and Safari up to 12.0.

To reproduce:

  1. With Safari + Voiceover, open the editor.
  2. Navigate to the tags box.
  3. Use VO + Arrow keys to focus on the tags combobox.
  4. When you type to add text, it is not added to the box.

The existing workaround is to navigate to a nearby element and use tab/shift+tab to place the cursor in the combobox. Our own investigations suggest that this is related to AJAX as it works when the textbox contains placeholder text.

Attachments (7)

44966.diff (1.4 KB) - added by wbrubaker 5 years ago.
Patch for 44966
before.png (27.2 KB) - added by pento 5 years ago.
after.png (26.8 KB) - added by pento 5 years ago.
44966-rev1.diff (1.4 KB) - added by wbrubaker 5 years ago.
Revision 1 - changing indentation rather than removing p tags
44966-rev2.diff (1.4 KB) - added by wbrubaker 5 years ago.
rev 1 was not correct. trying again
44966-rev3.diff (1.5 KB) - added by wbrubaker 5 years ago.
Now with comment goodness
44966.2.diff (1.8 KB) - added by ryelle 5 years ago.
This approach removes the p tag & adds CSS to fix the display differences. I've tested visually in Chrome/Safari, and Safari + VoiceOver.

Download all attachments as: .zip

Change History (27)

#1 @afercia
6 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.9.9

@parbaugh thanks and welcome to Trac. I can reproduce this with Safari 12 and VoiceOver 8. Works correctly with Chrome and VoiceOver so seems specific to Safari + VO.

In my experience with screen readers, sometimes the caret is not correctly placed within an input field for weird issues in the markup itself, often related to space collapsing. No JavaScript or CSS involved. I've just tried removing the <p> element that wraps the hidden label and the input and that fixed it for me. I was able to VO + Arrow keys and have the caret correctly placed within the field.

Another thing that seems to work is changing the indentation in the markup: currently it's <p><input ... and placing the input in a new line worked for me. @parbaugh would you like to check if these workarounds fix the issue for you too, when you have a chance? Thanks.

Moving to the 4.9.9 milestone, as it seems a very simple fix.

#2 @parbaugh
6 years ago

Thanks, @afercia. Both of the workarounds you listed worked for me.

#3 @konainm
6 years ago

@parbaugh @afercia

This problem can also be fixed by adding placeholder text to the input box. Even a blank placeholder with a single space works.

Please see the modified code for the tags meta box below:

function tagsdiv-post_tag( $post, $box ) {
	$defaults = array( 'taxonomy' => 'post_tag' );
	if ( ! isset( $box['args'] ) || ! is_array( $box['args'] ) ) {
		$args = array();
	} else {
		$args = $box['args'];
	}
	$r = wp_parse_args( $args, $defaults );
	$tax_name = esc_attr( $r['taxonomy'] );
	$taxonomy = get_taxonomy( $r['taxonomy'] );
	$user_can_assign_terms = current_user_can( $taxonomy->cap->assign_terms );
	$comma = _x( ',', 'tag delimiter' );
	$terms_to_edit = get_terms_to_edit( $post->ID, $tax_name );
	if ( ! is_string( $terms_to_edit ) ) {
		$terms_to_edit = '';
	}
?>
<div class="tagsdiv" id="<?php echo $tax_name; ?>">
	<div class="jaxtag">
	<div class="nojs-tags hide-if-js">
		<label for="tax-input-<?php echo $tax_name; ?>"><?php echo $taxonomy->labels->add_or_remove_items; ?></label>
		<p><textarea name="<?php echo "tax_input[$tax_name]"; ?>" rows="3" cols="20" class="the-tags" id="tax-input-<?php echo $tax_name; ?>" <?php disabled( ! $user_can_assign_terms ); ?> aria-describedby="new-tag-<?php echo $tax_name; ?>-desc"><?php echo str_replace( ',', $comma . ' ', $terms_to_edit ); // textarea_escaped by esc_attr() ?></textarea></p>
	</div>
 	<?php if ( $user_can_assign_terms ) : ?>
	<div class="ajaxtag hide-if-no-js">
		<label class="screen-reader-text" for="new-tag-<?php echo $tax_name; ?>"><?php echo $taxonomy->labels->add_new_item; ?></label>
		<p><input data-wp-taxonomy="<?php echo $tax_name; ?>" placeholder=" " type="text" id="new-tag-<?php echo $tax_name; ?>" name="newtag[<?php echo $tax_name; ?>]" class="newtag form-input-tip" size="16" autocomplete="off" aria-describedby="new-tag-<?php echo $tax_name; ?>-desc" value="" />
		<input type="button" class="button tagadd" value="<?php esc_attr_e('Add'); ?>" /></p>
	</div>
	<p class="howto" id="new-tag-<?php echo $tax_name; ?>-desc"><?php echo $taxonomy->labels->separate_items_with_commas; ?></p>
	<?php elseif ( empty( $terms_to_edit ) ): ?>
		<p><?php echo $taxonomy->labels->no_terms; ?></p>
	<?php endif; ?>
	</div>
	<ul class="tagchecklist" role="region"></ul>
</div>
<?php if ( $user_can_assign_terms ) : ?>
<p class="hide-if-no-js"><button type="button" class="button-link tagcloud-link" id="link-<?php echo $tax_name; ?>" aria-expanded="false"><?php echo $taxonomy->labels->choose_from_most_used; ?></button></p>
<?php endif; ?>
<?php
}

I have also created a plugin that can be activated to push the above fix please find it on:
https://github.com/KonainM/NYU-Tags-Accessibility-Fix

Last edited 6 years ago by konainm (previous) (diff)

#4 @afercia
6 years ago

@konainm interesting, thanks. I'd rather recommend to be as clean as possible. An empty placeholder sounds like a small hack to me. Instead, I'd recommend to just remove the paragraph and adjust the margins.

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


6 years ago

#6 @konainm
6 years ago

@afercia I agree. Thanks for your input. Just wanted to ask, would the removal of p tag fix be a part of the 4.9.9 update?

#7 @afercia
6 years ago

Hopefully, yes :)

@wbrubaker
5 years ago

Patch for 44966

#8 @wbrubaker
5 years ago

  • Keywords has-patch added; needs-patch removed

@pento
5 years ago

@pento
5 years ago

#9 @pento
5 years ago

  • Milestone changed from 4.9.9 to 5.0

Thank you for the patch, @wbrubaker!

Just removing the <p> changes the margins, as can be seen in the screenshots I've attached.

Rather than adding new CSS to handle this, the simpler solution is probably change the output HTML indentation, as @afercia suggested as a second workaround in comment #1.

@wbrubaker
5 years ago

Revision 1 - changing indentation rather than removing p tags

#10 @wbrubaker
5 years ago

@pento thanks for the feedback :)
Revised patch added

@wbrubaker
5 years ago

rev 1 was not correct. trying again

#11 @afercia
5 years ago

the simpler solution is probably change the output HTML indentation,

I'd agree except for the fact that is an extremely fragile solution, as indentation might be changed in the future. It would need some inline comment at least. It's true we're trying to work around a browser / screen reader bug, but it would be great to try some more solid solution.

@wbrubaker
5 years ago

Now with comment goodness

@ryelle
5 years ago

This approach removes the p tag & adds CSS to fix the display differences. I've tested visually in Chrome/Safari, and Safari + VoiceOver.

#12 @pento
5 years ago

  • Milestone changed from 5.0 to 5.1

#13 @afercia
5 years ago

  • Milestone changed from 5.1 to 5.0.1

Can this be considered for 5.0.1 please? It's a very small change and makes accessible to VoiceOver users a very important publishing feature.

#14 @pento
5 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#15 @pento
5 years ago

  • Milestone changed from 5.0.2 to 5.0.3

#16 @audrasjb
5 years ago

  • Keywords commit added

This ticket is triaged in milestone 5.0.3 as 44966.2.diff looks fine. It will needs commit and backport to the related branch.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


5 years ago

#18 @desrosj
5 years ago

  • Milestone changed from 5.0.3 to 5.1

Since this falls outside the 5.0.3 scope, punting to 5.1.

This ticket was mentioned in Slack in #core by afercia. View the logs.


5 years ago

#20 @afercia
5 years ago

  • Owner set to afercia
  • Resolution set to fixed
  • Status changed from new to closed

In 44551:

Accessibility: Improve text input in the Tags meta box when using Safari + Voiceover.

Props parbaugh, konainm, wbrubaker, pento, ryelle.
Fixes #44966.

Note: See TracTickets for help on using tickets.