In reviewing this page as a logged in user:
http://drupal7.dev.openconcept.ca/node/2

I get labels used as text with no associated form:

<label>Filtered HTML:</label>

This is a problem for screen readers and is just bad use of HTML. Seems like a simple enough patch.

Files: 
CommentFileSizeAuthor
#59 Heading-UI-Label.png150.42 KBmgifford
#58 drupal8.label-no-form.58.patch2.42 KBsun
PASSED: [[SimpleTest]]: [MySQL] 48,742 pass(es).
[ View ]
#52 radios_n_checkboxes.png33.79 KBmgifford
#51 filter_label_use_v8-882666-51b.patch3.24 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 48,124 pass(es).
[ View ]
#51 filter_label.png109.37 KBmgifford
#41 lanuage-negotiation-form_BEFORE_883666-40_firefox-13.png94.53 KBandrewmacpherson
#41 lanuage-negotiation-form_AFTER_883666-40_firefox-13.png95.18 KBandrewmacpherson
#40 filter_label_use_v8-882666-40.patch1.78 KBandrewmacpherson
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch filter_label_use_v8-882666-40.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#32 filter_label_use_v7.patch1.81 KBmgifford
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch filter_label_use_v7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#19 filter_label_use_v6.patch865 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 26,210 pass(es).
[ View ]
#11 filter_label_use_v5.patch1.11 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 22,846 pass(es).
[ View ]
#8 filter_label_use_v4.patch1.11 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 22,846 pass(es).
[ View ]
#6 filter_label_use_v3.patch1.11 KBmgifford
PASSED: [[SimpleTest]]: [MySQL] 22,871 pass(es).
[ View ]
#4 filter_label_use_v2.patch951 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 22,848 pass(es).
[ View ]
filter_label_use_v1.patch952 bytesmgifford
PASSED: [[SimpleTest]]: [MySQL] 22,848 pass(es).
[ View ]
screen-capture-58.png164.68 KBmgifford

Comments

This isn't really a problem for screen-raeder users, as far as I am aware. But, you are correct, it is poor markup.

good to know it doesn't bug screen readers. WebAIM's WAVE didn't like it and ya it should be fixed.

Status:Needs review» Needs work

Can someone fix the issue title? No idea what it tries to say.

+++ modules/filter/filter.module 13 Aug 2010 20:39:01 -0000
@@ -1075,7 +1075,7 @@ function theme_filter_tips_more_info() {
+  $name = isset($format->name) ? '<h3>' . check_plain($format->name) . ':</h3>' : '';

What about the trailing semicolon? Sucks, no?

Powered by Dreditor.

Title:Filtered description is contained in a label but the label inappropriatelyFiltered description shouldn't use a label when not associated with a form
Status:Needs work» Needs review
StatusFileSize
new951 bytes
PASSED: [[SimpleTest]]: [MySQL] 22,848 pass(es).
[ View ]

Thanks @sun - friggin trailing colon. Sorry. The title should be clearer too I hope.

+++ modules/filter/filter.module 13 Aug 2010 20:39:01 -0000
@@ -1075,7 +1075,7 @@ function theme_filter_tips_more_info() {
-  $name = isset($format->name) ? '<label>' . check_plain($format->name) . ':</label>' : '';
+  $name = isset($format->name) ? '<h3>' . check_plain($format->name) . '</h3>' : '';
...
   return '<div' . drupal_attributes($attributes) . '>' . $name . theme('filter_tips', array('tips' => _filter_tips($format->format, FALSE))) . '</div>';

Sorry, I just realized that this entire surrounding logic is needless... so while being here:

- $format->name is required, i.e., always non-empty.

- We can therefore skip the $name variable assignment and move that heading directly into the returned markup.

- That said, themers would highly welcome it if we'd make the generated markup more understandable:

$output = '<div ...';
$output .= '<h3>' ...;
$output .= theme('filter_tips'...;
$output .= '</div>';
return $output;

Powered by Dreditor.

StatusFileSize
new1.11 KB
PASSED: [[SimpleTest]]: [MySQL] 22,871 pass(es).
[ View ]

Good suggestion. That's much more straight forward.

Status:Needs review» Needs work

Awesome!

+++ modules/filter/filter.module 16 Aug 2010 18:06:54 -0000
@@ -1074,11 +1074,13 @@ function theme_filter_tips_more_info() {
+  $output .= '<h3>' . check_plain($format->name) . '</h3>';  ¶

...except for the trailing white-space ;)

Powered by Dreditor.

Status:Needs work» Needs review
StatusFileSize
new1.11 KB
PASSED: [[SimpleTest]]: [MySQL] 22,846 pass(es).
[ View ]

S#!t, like where are all of these spaces coming from anyways.. cut & paste errors suck..

Thanks sun.

@Everett, via twitter @webaxe said "Besides poor code, orphaned labels may mislead user; expected functionality broken such as clicking to activate a form field", although he followed up by saying "It ain't a show stopper."

So would be good to fix it, but not critical.

Status:Needs review» Needs work

Something odd happened to the patch in #8... very odd. Saw the proper file contents yesterday.

Anyway, there was still trailing white-space contained, and I hoped you'd recognize it yourself...

Status:Needs work» Needs review
StatusFileSize
new1.11 KB
PASSED: [[SimpleTest]]: [MySQL] 22,846 pass(es).
[ View ]

Ok, I visually reviewed this patch in another editor. Not sure how that space was missed using vi......

Status:Needs review» Reviewed & tested by the community

Thank you! :)

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

Status:Fixed» Closed (fixed)

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

Status:Closed (fixed)» Active

This turned into a regression, #890212: text format help text uses wrong headings

LABELs do not follow and cannot break any hierarchical document structure. H3 does.

As mentioned in #884846: Orphaned Labels Being Generated for Radios, Checkboxes & Items, we should be using a non-hierarchical element, such as DIV.form-label or similar.

For accessibility headings help to break of the navigation of a site & allow screen readers to have an alternate way to skip through the content looking for chunks of relevant text.

Generally navigation is done either by reading out the text of the site chronologically or either skipping through the links or through the headings. Although DIVs won't break the hierarchical document structure they don't add anything either.

It's better to use a DIV than a LABEL in this instance, as a LABEL is just bad HTML, however headings provide a structure that can be navigated. I think that these two 456 Berea Street articles Heading & Document Structure & Headings & Heading Hierarchy sum it up pretty nicely.

* Use headings for anything that looks or “feels” like a heading.
* Use headings to identify what WCAG 2.0 calls “sections of content”, i.e. headings in the main content area, different navigational sections, etc.

There isn't right now a nice way to deal with this. Hopefully this will change with HTML5.

Thanks for the additional insight. Since we can't use HTML5's section yet, we need to change it into a DIV, since headings receive different styling depending on their level. All of those semi-labels that may appear in forms or with or without an actual form element should visually look like a form element label, but not like a heading of an arbitrary level.

In this case, it seems like the semi-label is sometimes appearing in forms and sometimes not. I guess it would make sense if we'd change the other patch to use DIV.label instead of DIV.form-label then, and also use DIV.label here.

Should be a job of a minute.

If it was just this patch it would be quick. However there are a few places where I know this has been done. Not sure how to best find them though.

There's is a great deal of disagreement about how to use heading tags in HTML4. This doesn't need to wait till HTML5 or for that matter adoption of WAI-ARIA.

I'll see if I can get some second opinions about this.

Status:Active» Needs review
StatusFileSize
new865 bytes
PASSED: [[SimpleTest]]: [MySQL] 26,210 pass(es).
[ View ]

Ok, I'm going to concede on the on heading issue and run with the div after discussing this this with @BrandonOJC.

We're going to also need to re-roll to be either a .label or a div.label:

.form-label {
  display: block;
  font-weight: bold;
}

from http://drupal.org/node/884846#comment-3347986

Hope this helps move this issue forward.

EDIT: Ultimately I suppose it comes down to what users expect. Headings within forms isn't standard even if a group of form elements is essentially the same as a paragraph or two.

@sun can we get this RTBC'd so that we can then build against it when modifying #884846: Orphaned Labels Being Generated for Radios, Checkboxes & Items to use:

<div class="label"></div>

As mentioned over there, the first patch that introduces DIV.label also has to introduce the corresponding stylesheet changes, which are currently contained in the patch in http://drupal.org/node/884846#comment-3347986, but are still using .form-label, not .label.

sub

Ok, I've re-rolled that patch & submitted it.

#19: filter_label_use_v6.patch queued for re-testing.

Is this the CSS change we need to make?:

-.form-item label {
+.form-item label, div.label {
   display: block;
   font-weight: bold;
}

Or can we get rid of .form-item label and just make it only div.label?

This corresponding change would be made in system.css, bartik/css/style.css, and seven/style.css.

Hey Brandon, It's in this patch here http://drupal.org/node/884846#comment-3499784

But yes, I don't think we can get rid of the .form-item label as we want the labels to be defined to look the same. The patch I rolled didn't include a change to the system.css

mike@dev:/home/dm7$ grep -ir form-item modules/system/* | grep label
modules/system/system.admin.css:.exposed-filters .form-item label {
modules/system/system.admin-rtl.css:.exposed-filters .form-item label {

This is the relevant code:

.exposed-filters .form-item label {
  float: left; /* LTR */
  font-weight: normal;
  width: 10em;
}

It looks like div.label should be applied there too.

#19: filter_label_use_v6.patch queued for re-testing.

Version:7.x-dev» 8.x-dev

Bumping to D8. This is important to resolve, since it is poor practice to use label in this way, but it has no real impact on users, only on testing tools, so it can wait for D8 to be resolved.

Ok, so how do we get these in for consideration in D8. They are pretty trivial accessibility enhancements.

Issue tags:+html5

This isn't an accessibility issue as such, but it does cause confusion as it isn't actually a label.

Would be a lost opportunity too to remove this as we're going through & verifying HTML5 coding standards.

A grep for '' in D8 just produced:

core/modules/field/field.form.inc:        'data' => '<label>' . t('!title: !required', array('!title' => $element['#title'], '!required' => $required)) . "</label>",
core/modules/field/modules/text/text.js:          $fullLabel = $('<label></label>').prependTo($full);
core/modules/locale/locale.admin.inc:    $title = '<label>' . $form[$type]['#title'] . '</label>';

The instance has since been removed from:
/core/modules/filter/filter.module

Status:Needs review» Needs work

Patch is from CVS days.

StatusFileSize
new1.81 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch filter_label_use_v7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Good point.. Hopefully this moves it ahead a bit.

Title:Filtered description shouldn't use a label when not associated with a formCore form descriptions shouldn't use a label when not associated with a form
Status:Needs work» Needs review

Oh ya, I also hit up the following files.

filter/filter.module field/field.module locale/locale.admin.inc system/system.admin.css

Needs before/after screenshot and UX review.

Assigned:Unassigned» Bojhan

Waiting for screens.

#32: filter_label_use_v7.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Needs usability review, +accessibility, +html5

The last submitted patch, filter_label_use_v7.patch, failed testing.

Status:Needs work» Needs review

Patch didn't apply when I tried it...

error: core/modules/locale/locale.admin.inc: No such file or directory

Status:Needs review» Needs work

Sorry, status field was playing games with me :-)

Status:Needs work» Needs review
StatusFileSize
new1.78 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch filter_label_use_v8-882666-40.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Okay, I re-rolled patch. It turns out that a some of locale.module is now in language.module...

Language negotiation system now part of language module

Screenshots before and after the patch in comment #40.

I haven't done a thorough test across browsers yet, but the new DIV is less prominent than the LABEL in my Firefox 13 test.

I hope I find this issue before.

Here is same problem:
Point #5
http://drupal.org/node/1452188#comment-6115332

I had forgotten why we had removed <h3> until stumbling down to comment #15.

Any suggestions on dealing with the DIV? Should we just make it larger in the CSS?

Nothing to review, other than a markup bug?

Issue tags:-accessibility, -html5

#40: filter_label_use_v8-882666-40.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+accessibility, +html5

The last submitted patch, filter_label_use_v8-882666-40.patch, failed testing.

Status:Needs work» Needs review

@Bojhan, in the screenshots in #41 div.label looks less bold than the <label> tag. I can't see why that is, but think that's why it needed a usability review.

What was the markup bug? Would like to get this marked RTBC.

Well I imagine we don't want to change anything visually, heh :D that's the markup bug.

Status:Needs review» Needs work

Is it standardize all help texts headings to div.class=label ?? why not use h2~h6 tag or dfn

+++ b/core/modules/language/language.admin.incundefined
@@ -567,7 +567,7 @@ function theme_language_negotiation_configure_form($variables) {
+    $title = '<div class="label">' . $form[$type]['#title'] . '</div>';

Assigned:Bojhan» Unassigned

Status:Needs work» Needs review
StatusFileSize
new109.37 KB
new3.24 KB
PASSED: [[SimpleTest]]: [MySQL] 48,124 pass(es).
[ View ]

@droplet see @sun's comment here - http://drupal.org/node/882666#comment-3402358

StatusFileSize
new33.79 KB

It's worth also noting that this doesn't fix the same problem with radios & checkboxes.

This is just on the page creating new users.

Edit: I don't think #884846: Orphaned Labels Being Generated for Radios, Checkboxes & Items has a solution that we can work with easily to get rid of these headings.

#51: filter_label_use_v8-882666-51b.patch queued for re-testing.

Issue tags:+a11ySprint

This is a simple fix.

Status:Needs review» Reviewed & tested by the community

@droplet it might be more semantic to change this to a heading, but essentially this issue was opened because it was just a markup bug for an inappropriate use of a label element.

I'm fine with someone else verifying that indeed it is appropriate for it to be a heading & making it so, but I'd really like that to either be another issue or to to have someone else champion it.

This addresses the original concern I raised in 2010 and I'm marking it RTBC as it's really a trivial markup change that shouldn't take 2 years to solve.

Note that I'm currently working on #1829202: Make #type 'item' work outside of a form context to render a compound label + content

Essentially, all of the cases in this patch try to output a container that is compound of a label + inner content, but they are not necessarily in a form context, or there is no corresponding input element for the label (which really means it is just a heading).

Status:Reviewed & tested by the community» Needs review

I'm going to drop this down to needs work until I understand how your other patch is going to interact here.

In your patch does it become just a heading or is it a label that is styled to look like a heading?

StatusFileSize
new2.42 KB
PASSED: [[SimpleTest]]: [MySQL] 48,742 pass(es).
[ View ]

Yeah, #1829202: Make #type 'item' work outside of a form context to render a compound label + content, #type 'item' essentially becomes just a heading (H4) + arbitrary content markup.

However, I just checked this patch here once again, and it does not really overlap with that issue. It would merely make sense to harmonize the two in terms of markup + CSS.

Attached patch does so. Tested manually, and still works as expected. So I'd consider this back as RTBC.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new150.42 KB

Here's a screenshot from FF in Ubuntu.

Thanks @sun.

Status:Reviewed & tested by the community» Fixed

Looks fine. Committed/pushed to 8.x, thanks!

That's great, thanks!

Status:Fixed» Closed (fixed)
Issue tags:-accessibility, -html5, -a11ySprint

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