So when editing this page admin/people

There are some labels missing. I caught one in this patch, but the checkbox was eluding me.

Anyways, we need to set up appropriate labels here.

CommentFileSizeAuthor
#6 user_admin_labels_v3.patch597 bytesJeff Burnz
#2 user_admin_labels_v2.patch632 bytesmgifford
screen-capture-61.png230.65 KBmgifford
user_admin_labels_v1.patch633 bytesmgifford

Comments

Jeff Burnz’s picture

Jeff Burnz commented
Status: Needs review » Needs work 
+++ modules/user/user.admin.inc	15 Aug 2010 18:29:01 -0000
@@ -176,6 +176,8 @@ function user_admin_account() {
+    '#title_display' => 'invisible', ¶

whitespace

Powered by Dreditor.

mgifford’s picture

mgifford
he/him
Primary language English
commented
StatusFileSize
new user_admin_labels_v2.patch632 bytes

whitespace removed, thanks!

Everett Zufelt’s picture

Everett Zufelt commented
Status: Needs work » Needs review

bot?

mgifford’s picture

mgifford
he/him
Primary language English
commented

Thanks. Seems I missed this in a few of my Label patches. Now it just needs to be marked RTBC by someone.

Everett Zufelt’s picture

Everett Zufelt commented
Status: Needs review » Needs work

I think you intended to attach a patch here.

Jeff Burnz’s picture

Jeff Burnz commented
Status: Needs work » Needs review
StatusFileSize
new user_admin_labels_v3.patch597 bytes

Shouldn't the text read "Bulk update options for user"?

Status: Needs review » Needs work

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

Jeff Burnz’s picture

Jeff Burnz commented

I dont understand the fail.

mgifford’s picture

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

#6: user_admin_labels_v3.patch queued for re-testing.

mgifford’s picture

mgifford
he/him
Primary language English
commented

I don't understand SimpleTest well enough, but reading through modules/field/tests/field.test I'm not sure how adding a label to the user admin page is throwing the error to function testFieldFormUnlimited().

Everett Zufelt’s picture

Everett Zufelt commented

This looks like it's passing now. Are we good to be RTBC?

mgifford’s picture

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

Yes..

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Closed (duplicate)

wow. Please don't get me wrong. I'm proof-reading all RTBC patches right now, and thinking back to what I already processed, there are a lot of similar patches in the queue already, so instead of reviewing other important patches, I'm spending time to review the very same change (but in a different location) all over again. So please don't get this rant wrong:

Seriously. Why do we need to have like gazillions of separate issues that fix #title/#title_display properties in different modules throughout core?

Please merge this patch + all the others into a single one.

Thanks.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Closed (duplicate) » Reviewed & tested by the community

@sun I do think it's easier for folks to look at in bite sized pieces. It's impressive that you've gone through and reviewed so many patches marked RTBC. Thanks.

I do appreciate that there are issues with trying to get so many of these annoying little label patches in core. I realize that it must be frustrating to go through and review them all, but I'm not sure that a master patch is really going to be any easier to approve.

Since this hits so many different components & different folks are more familiar & hold responsibility for different components I'm setting this back to RTBC. It's a very trivial change. It's been reviewed by a bunch of folks. Making a mega patch isn't going to make it any easier to test, understand or ensure that it is really saying what the field is doing.

That being said, I appreciate that it's really annoying & wish there was an easy way to separate this off. Maybe we could set a difficulty level for the patch. Hard, Normal, Easy, Super simple.. Most of the label changes are pretty simple, but they aren't trivial for accessibility.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Closed (duplicate)

@mgifford: The problem is that we are discussing the very same implementation details and oddities in all of those issues all over again. A single patch can happily hold all changes, and you can provide a list of URLs/pages to check. What all of those issues effectively do is a serious slowdown and delay of the Drupal 7 release.

One of those issues was marked RTBC even though it contained a blatantly wrong change: #882694: Add missing form element titles for accessibility

A couple of those issues are using hidden #title properties that don't really make sense to me, even though I can see the form elements.

All of this calls for a centralized discussion. Separating every single change out into a separate issue is contradictory to the entire goal.

So pretty please mark all of those issues as duplicate and do a master patch. #882694 looks to be the largest of them, so it might be a good idea to re-use that issue.