Code cleanup: 'authenticated users' role

+1. It follows the KISS rule.

A screenshot maybe? It helps to show this to the others ... :)

Your patch disables them all. Rest seems fine.

I like this very much, and this is a must-have, not just a nice little feature. In fact, I think this is a usability bug, as the 'authenticated users' has been (automatically) assigned to the user, but is concealed from the admin.

Still, Gurpartap is right: Your patch in #2 disables all checkboxes.
This is because you assume the FormAPI extension in #104715 to be committed. Unfortunately this has been definitely postponed to D7.

After some fiddling I came up with a solution for D6 that doesn't rely on the FormAPI extension. It doesn't exactly result in beautiful code, but it's the only way to do it right now. This demonstrates how much that small FormAPI extension is needed to produce lean code.

Okay. But the good news is: it finally works, tested well and makes the behavior of roles more intuitive. No string changes involved.

Remember that this should (and can) be changed to nicer code in D7, as soon as #104715 has been committed.

Oops, I overlooked that... I'd let the string in, we can change it for D7, when we have to touch that code again, anyway...

Luckily it is not a string freeze violation to *remove* strings. To add is a violation and to modify is an evil violation.

+1 This is great! What else should I say? It works. And it improves usability a lot. RTBC.

After committing please keep open! After #104715 has been committed to D7, we can simplify the usage of FormAPI.

We still need some feedback, please!

Well, make sure to add a code comment on the unusual (re)use of the 'checkboxes' form API construct. If I understand it right, you are adding a subelement to the form item, which will be part of the list of generated elements for that checkbox list. If this gets documented right, this looks like a good UI improvement.

Didn't apply anymore. Rerolled with a much more detailed code comment on this rather unusual FormAPI usage.
If the comment is not too long now, I guess it should be RTBC.

Thanks, committed.

Reopening this for D7. The rather ugly workaround should be replaced by more straight-forward code, after #104715 has been committed to HEAD (D7). I will post a patch after that has happened.

Moving all usability issues to Drupal - component usability.

Not relevant anymore...

Comment #21 was (and is) definitely still relevant - the ugly workaround is still there. However, it looks like that issue has been moved to #284917: Allow FAPI select, radios, and checkboxes to specify some options as disabled. I'll leave a comment there to make sure it fixes this code as part of that patch.

Moving this issue back to 6.x and leaving it closed.

Nope, this belongs to the 8.x queue. The underlying FAPI-deficiency (#284917: Allow FAPI select, radios, and checkboxes to specify some options as disabled) is unfortunately still not solved. It might be backported to 7.x afterwards.

There's really no reason to keep two issues open for the same thing, and the original bug here was fixed long ago.

Let's mark it postponed if you must leave it open, though. But I think it will be fixed just fine by the other issue.

It turns out this always has been possible.
So here's a followup removing the cludgy workaround.

Another try

Oh my goodness.
If it's the "missing newline" bug that I keep running into, then this patch should work fine even for the picky testbot:

currently a minor task

This is now where discussion about options is happening #342316: Introduce proper Form API #types for 'option' and 'optgroup', and make #options consistent..

Patch no longer applies:

error: core/modules/user/lib/Drupal/user/AccountFormController.php: No such file or directory

// Special handling for the inevitable "Authenticated user" role

Comment needs a full stop at the end.

Really though, shouldn't followups be their own issues? I'd be tempted to re-close this if there wasn't a patch waiting.

I'll look into re-rolling this.

Re-rolling the patch from #31.

wrong patch

Sorry, submitted the wrong patch. Second one works.

+++ b/core/modules/user/src/AccountForm.php
@@ -213,7 +201,12 @@ public function form(array $form, array &$form_state) {
+    );
+
+// Special handling for the inevitable "Authenticated user" role
+    $form['account']['roles'][DRUPAL_AUTHENTICATED_RID] = array(
+      '#default_value' => TRUE,
+      '#disabled' => TRUE,

The comment is not indented and does not end with a period. Pls correct this.

Corrected, Thanks.

Triggering testbot.

Committed 945ed20 and pushed to 8.0.x. Thanks!

Creating back port

Backported & tested on 7.34

Updating version to 7.x-dev in order to test my backport.

#45 looks good to me. The code changes are almost completely identical to what went into D8. The HTML generated doesn't seem to have changed at all either.

Before:

After:

I'll RTBC it.

Hm, I'm not sure we should do this in a stable release.

It's certainly the "right" thing to do, but the user account form is something people heavily form-alter, and in this patch we are changing the data structure (granted only a little bit, but still changing it in a way that could break something).

Given that it's not a functional bug (as far as I understand), just a code cleanup, maybe we shouldn't bother?

Backported & tested on 7.44