The red * to denote a required form field is really an abbreviation for the text "this field is required". It seems to me that the semantics would be better expressed if the span element was an abbr element. Patch attached.

CommentFileSizeAuthor
#22 Required marker original42.25 KBliam morland
#22 Required marker patched43.12 KBliam morland
#22 Tablesort original88.29 KBliam morland
#22 Tablesort patched88.29 KBliam morland
#17 asterisk-abbr.patch8.21 KBliam morland
#11 required-abbr.patch5.29 KBliam morland
#7 required-abbr.patch3.23 KBliam morland
required-abbr.patch429 bytesliam morland

Comments

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, required-abbr.patch, failed testing.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Does anyone know why this failed? Is it testing for a specific string in the required message, but that string is different on purpose because of this patch?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Yes, exactly.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Thanks for your reply. In that case, the patch is fine, even though it fails the test. Is there something I need to do to get this patch reviewed by the appropriate people and it either committed or decided that this change won't be made?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Well for it to be committed, the patch needs to update the test files so that the tests pass again.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
StatusFileSize
new required-abbr.patch3.23 KB

Thanks. This new patch version patches both the function and the tests.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Needs work » Needs review
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

I just found this on w3schools:

The < abbr > tag is not supported in IE 6 or earlier versions.

So as long we support IE6 we can't submit this as is. Also, I don't know if < abbr > is really the right tag here.
The example on w3schools is:
<abbr title="World Health Organization">WHO</abbr>

Hmm...

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

On the other hand, that just means, that IE6 won't style this element, i.e. it won't be red, as it will in all other browsers. Don't know if that's acceptable in terms of progressive enhancement. It's not really unusable...

Hmm... :)

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
StatusFileSize
new required-abbr.patch5.29 KB

Here is an updated patch, rolled against the latest D8 and including CSS changes which mean that the patch creates no visual changes.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Issue tags: +Accessibility
Everett Zufelt’s picture

Everett Zufelt commented

@Liam

I'm still not completely certain that we need this, as I feel like * is universally understood to indicate a required form field.

However, if it doesn't change appearance then this is more semantic, and cannot hurt accessibility.

I would like us to role all usages of * into the same patch, it is currently used to indicate unsaved changes on blocks that have been moved on the Block administration page, perhaps it is for all table rows moved with tabledrag. This would ensure consistency.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

I think we need some form of proof or usage-example or whatever, that <abbr> actually is semantically more correct here.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Using abbr around a "required" asterisk comes directly from Techniques for WCAG 2.0, "H90: Indicating required form controls", Example 2.

Abbreviations don't have to consist of letters. Just as a11y is an abbreviation for accessibility and § is an abbreviation for section, * is an abbreviation for "this field is required".

Everett Zufelt’s picture

Everett Zufelt commented

@liam

This gets a +1 from me, provided that we roll all of the uses of * into a single patch for consistency.

See #925862: Asterisk is not semantic/accessible on re-ordered table rows

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
StatusFileSize
new asterisk-abbr.patch8.21 KB

Here is a new patch that takes care of all uses of *.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Title: Required marker * should be in abbr not span » Asterisk * used for required or changed marker should be in abbr not span
mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Needs review » Needs work

That's nice and simple. Applied it and it works great!

The only thing that stopped me from marking this RTBC is this part of the diff:

-  // except for form elements of type 'text' and 'textarea', where the 
-  // spacebar activation causes inappropriate activation if #ajax['keypress'] is 
+  // except for form elements of type 'text' and 'textarea', where the
+  // spacebar activation causes inappropriate activation if #ajax['keypress'] is

Isn't this identical?

Any reason not to backport it to D7? There are no text changes. Some possible theming changes though for some themes, so perhaps it will have to stay as a D8 enhancement. Looks like it could be fixed in the Accessible Helper module though.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

There were a few lines in the original code with trailing spaces. My editor stripped them automatically, so they show up in the diff. I could role a patch without those removals if needed.

It is possible that themes may style the asterisks with selectors that use the span, so this is probably best as part of the Accessible Helper module.

Everett Zufelt’s picture

Everett Zufelt commented
Status: Needs work » Needs review

Good that the trailing whitespace was removed, that is difficult to identify when visually inspecting the patch. Definitely cannot be backported to D7.

If we can get some before and after screen-shots to show that there are no visible changes we can mark this RTBC

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
StatusFileSize
new Tablesort patched88.29 KB
new Tablesort original88.29 KB
new Required marker patched43.12 KB
new Required marker original42.25 KB

Before and after screenshots attached.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

The tiny shift on the required marker screenshots is due to my mistake when I cropped the images.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Excellent. Thanks for this Liam!

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Corresponding D7 Accessible Helper Module issue: #1091622: Asterisk is not semantic/accessible on re-ordered table rows

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Looks good. Committed this to 8.x. I'm not sure we want to backport this to 7.x as it could break themes.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Liam, any thoughts as to whether we could include this in the Accessible Helper Module http://drupal.org/project/accessible

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

There is now a patch underway in #1091622: Asterisk is not semantic/accessible on re-ordered table rows.

Automatically closed -- issue fixed for 2 weeks with no activity.