Usability: Auto-check permissions if "authenticated" has them

The introduction of jQuery means some rework

Apllied and tested. I like this patch. Simple and neat and the visually effective. So it gets a +1 thumbs up.

But getting it into Drupal 5, well, we're in code freeze so strictly speaking this (albeit neat) patch is a feature. That's the only reason I haven't set it RTBC. If you can find a kind hearted Core Comitter you can persuade, good luck!

regards,
--AjK

Before applying the patch, I had an "administrator" role with "administer nodes" permission checked.

After applying the patch, that permission got cleared. So marking code needs work.

I think this does make things more intuitive, although I'm concerned that this might make the authenticated user permissions stored once per each role in the permission table, and I'm not sure what kind of performance issues that might cause when checking permissions and such.

Awesome!! Seems to work well and is a nice improvement. :)

For the record, it now disables the checkboxes rather than check them.

Not going to commit this unless there is much more demand for it. Personally, I don't think it is particularly useful.

I +1'ed this because 3 different people on 3 occasions have told me that they didn't realize the permissions on this page were additive; that is, when you have "create forum topics" and "edit own forum topics" checked for authenticated user, and also want people in the "administrator" role to have those permissions, you don't need to check it for both of them.

I remember not knowing this too when I was first starting with Drupal, and making sites with the permissions matrix like:

            [anon] [auth] [role1] [role2]
[some perm]   X      X      X       X

When all I needed was

            [anon] [auth] [role1] [role2]
[some perm]   X      X    [ ]        [ ]

We can improve the documentation in the help hook above this section (and probably should), but kkaefer's patch makes it impossible to mess this up, so gets my +1. :)

Dries, yes, it's not that useful to the experienced user but small UI improvements like this for the new adopter / novice user just makes that learning curve slightly easier to get up. And we all know it's not the shallowest of curves.

So, as I had previously said, I like this patch (and any like it that make the UI more obvious, intuative and easier to use).

regards,
--AjK

This enhancement seems generally useful. I've read over but not applied the patch. The js coding is clear and elegant.

Should some of this filtering be done on the server side, in a _validate hook, to cover the case of no javascript?

Seems quite useful to me as well. I remember when I first started with Drupal I didn't realize the permissions were additive like that, so +1 for that.

Big +1 from me.
It is this kind of small, jQuery-powered, nicely degrading usability enhancments what JavaScript is great for.
Tested on HEAD, still applies, works as advertised.

For me it's RTBC.

nedjo: no, if javascript is not enabled, it behaves as it did before (iow the javascript is does not actually check the boxes (because the permissions are there anyway, because they're additive).

I'd like to brainstorm about this some more, before it gets committed.

What is a common/good way to communicate to the user that permissions are additive (other than writing text)?

Apparently Dries thinks that thi sstill needs work.

Patch still applies.

I like this very much, but we'll have to revisit permissions UI altogether. Too late for D6 anyway.

Moving all usability issues to Drupal - component usability.

This tripped up three participants in the recent usability testing, so it needs a fix. I think the problem is more complicated than just checking and disabling checkboxes. If I check the authenticated user box by mistake, and it automatically checks and disables all the boxes to the right of it, my instinct might be to start unchecking boxes. When I can't, I'm going to be frustrated.

<brainstorming>I'm not even sure checkboxes are the right widget here. When you're assigning permissions, your real choices are

o All site visitors
o All logged-in users
o Only certain logged-in users
  _ admininstrator
  _ forum moderator
  _ content manager
  _ (whatever roles you have on your site)
o Only the superuser

If we didn't have such tight space constraints on the permissions page, we could use a set of radio buttons for each permission, with a set of checkboxes appearing below the "Only certain logged-in users" radio button. Given the space constraints we have, though, I don't know what the right answer is.</brainstorming>

If we do write JS that auto-checks permissions, it needs at a minimum to remember what state the checkboxes were in before the "Authenticated users" box was checked, in case the user changes his mind and unchecks the auth users box.

I think @ksenzee is exactly right here. This seems like a case where many of the possible "fixes" could actually cause more problems than they solve. I guess disabling the checkboxes might sort of work, but I definitely don't think we want to check them while disabling them. All that is going to do is reinforce whatever misconceptions people have that led them to believe they needed to click on the checkbox in the first place.

I think the brainstorming is good and makes sense, but do we know more from the usability testing about the exact nature of people's misconceptions here? It seems to me like there are at least two different possibilities:

1. People might not understand that Drupal lets users have more than one role, so they might think that when they make someone an "administrator" that person actually loses the "authenticated user" role, and therefore they need to assign the permission to both.

2. People might interpret an unchecked checkbox as meaning "deny this permission from any user with this role" -- therefore, they check it with the goal of preventing that from happening.

Speaking from experience, I know this definitely tripped me up when I started using Drupal, but I don't remember why :) I think my confusion was probably more along the lines of #2 than #1, though.

Patch is updated for D7.
It now is integrated into the user.js file.

Behavior:
When an authenticated user permission is checked, all corresponding custom role permissions are checked and disabled. @Dries There seems to be general agreement that auto-checking and disabling checkboxes is the clearest way to communicate that these permissions are inherited.

If a custom role permission was checked and then the authenticated permission is checked and unchecked, the original state of the custom role permission is preserved. @ksenzee This feature has been in this patch since 2006 and addresses your concerns.

When the form is saved, the disabled checkboxes don't get stored into the role_permission table. This is nice because it keeps the list of permissions as concise as possible.

Without javascript, the form behaves as before.

@David_Rothstein: just updated this while you posted. I think disabling AND checking is clearer to me. Most people really don't care about the details of how the roles work- they just want to be satisfied that the role has the right permission and seeing the box checked seems more reassuring than if it is simply disabled. Can you try out the new patch and see what you think?

We will need a definite usability review on solutions proposed here.

I tried out the new patch. It works better than I expected it would. But I still think it's going down the road of misleading people rather than educating them :) I would definitely like to see a review from the experts on this one, yes...

Also, I did find another scenario where this patch can lead to even more confusion. Steps to reproduce:
1. Create a role called "administrator".
2. Go to admin/user/permissions, grant authenticated user the "Create Article content" permission and be comforted by the fact that it checks the corresponding box for administrators :) Submit the form.
3. Now go to admin/user/roles and click the "edit permissions" link next to the administrator role. Notice that the "Create Article content" checkbox is not checked on this page..... "Wait a minute, I swear that box was checked on the other page! How come it's not checked here? Do my administrators have this permission or not?"

I was just thinking about that per-role permissions page problem as well. Even without this patch, most of the same problem exists- you can't see that the role has a certain permission that auth has, so you are forced to click a bunch of redundant checkboxes or else give up and look at the full permissions page.

I think we should add disabled checked boxes to the per-role permissions page as well.

As far as whether it's misleading them rather than educating, I disagree. Leaving the boxes unchecked means that in order to infer that a role has a permission you have to both realize that you need to check the authenticated user column and also do so to examine every permission. This means your eye has to leave the permission you're looking at, go look back at the table headers, and track back down the authenticated column just to answer the question of whether role x actually is granted permission y. I think this adds a lot more clarity than it does mislead people.

@Jody Lynn: I see where you're coming from, but in the scenario you describe, role x actually is not granted permission y. It just so happens that every user who has role x also has a different role z ("authenticated user") that grants them the permission. If you managed to take role z away from these users, they would not have the permission, even if they were still in role x...

How much mileage could we get here just out of renaming "authenticated user" to "All logged-in users" or maybe "Any logged-in user" (this was actually part of @ksenzee's brainstorming in #23). That doesn't solve the whole problem but it's a much lower-risk change.

[my comments keep disappearing]

I added js for the per-role permissions page to check and disable permissions which are granted to the authenticated user role.

It feels a lot easier to use to me.

A disabled checkboxes tells the user 'you can not use this'. But that is not what we want to tell, we want to say 'you can leave it, there is no need to use it'. This leaves open the option to still check the checkbox for a good reason, for example for an admin role. Let the user free and don't force a situation upon him.

My suggestion is to add a red or green color to the backgound of the checkbox to indicate the permission status as a result of the checkbox status. The attached screenshot gives an impression.

What ever solutions we come up with, only user testing can tell us if it is truly usable.

I think adding colors to this page,which is working quite good right now - is going to be a major visual problem (clutterd). Can we have a demo up on what the latest patch does? #31

I think #34 has the right theory behind it, but one problem with the green/red is that those colors can be misinterpreted. Red could easily mean "Error, don't click me!", whereas green can mean "OK, please click me!", which is almost the exact opposite of the message we're trying to send...

It may just be that there is no way to solve this without introducing some kind of textual change somewhere. At some point, an idea becomes too complicated to explain to people without using words... Would it be ridiculous to suggest using something like a tooltip here? One nice thing about a tooltip is that it would only appear when someone is on the verge of/already is clicking the particular checkbox. So it doesn't clutter up the page at all, but rather only appears for users whose behavior suggests they might need it.

Possible text could be something along the lines of "Users with the ___ role are all authenticated and already have the _____ permission" although that may be too verbose.

I would simply go with checking and disabling all implied permissions.

"You can leave it, there is no need to use it" is not at all what should be conveyed. There is no good reason to have the user make a decision that does nothing.

The form should bridge the user's mental model and Drupal's system model. All roles have the permission if authenticated user has the permission, those checkboxes should be checked. Unchecking those implied permissions is not useful, so they should be disabled.

"You can leave it, there is no need to use it" is not at all what should be conveyed. There is no good reason to have the user make a decision that does nothing.

I don't think it's correct to say that the checkbox does nothing. Clicking on the checkbox is how you tell Drupal "I want to make sure that people in the [administrator] role have this permission even if someone later takes it away from authenticated users." So for example, if someone later comes along and edits the permissions for authenticated users (especially via the per-role page, admin/user/permissions/2, where it's out of context), their edits will not remove the permission from administrators.

(As a rough analogy from the real world, a lot of local governments have laws on the books which guarantee certain rights to their citizens, even though the national government already guarantees those rights separately, etc.....)

+1 for this.

@kkaefer: That last bit is probably quicker to commit in a separate issue ;)

That last bit is actually especially quick, since that's the way Drupal already works :)

It's been that way since Drupal 6, as far as I know.

This would indeed be great. While it wasn't serious bug in Baltimore testing, a few people didn't seem to grasp permissions inheritance on that page, so tagging.

Just checking and disabling the checkboxes ought to work no?

Looks nifty and avoids permissions table bloat which is a handy byproduct.

catch explained me how this works, and it looks good. The path lacks some context, as the roles are cut off in the screenshot - even though I always try to advise against checkboxes being faded out, it seems that in this case it might actually make sense.

Looks good, but kkaefer's explanation in #44 should be added as code comments. Like that, we can figure why we used dummy checkboxes 3 years from now. Let's get this in! :)

Looks great, thanks for adding documentation. Committed to CVS HEAD. Thanks.

Dries/webchick - I'm setting this back to RTBC because user.permissions.js from #48 didn't make it into the commit. I don't know if you want to go ahead and commit as is, or set back to CNW (see below).

kkaefer #50, good call. I just tested in IE 6 and 7 and it's definitely broken; the change event doesn't fire until you click outside the checkbox. So if you check an auth user checkbox and then try to check the box to its right, it feels like the click on the second box has disabled the whole row of boxes. Happily, the click event does work as expected, and it gets fired even on keypress in FF3, IE6 and IE7. I think we can safely bind to click instead of change.

In IE6, we have the additional problem that setting .attr({checked: true}) in line 20 doesn't actually check the dummy box. We'd have to add "checked='checked'" to the HTML in line 17 if we want to support IE6. Do we have a decision on IE6 support in D7 yet?

I should have time to rework this (and test in Safari and FF/Mac) on Wednesday, unless someone beats me to it. :)

I committed the missing file (version in #52), even though it might be a bit broken still. It would be good if someone with IE could get CVS HEAD, and mark this as fixed when everything works. Sorry for missing that file, and disturbing the regular workflow. ;-)

I tested the newest version on a bunch of Linux/Windows browsers (including IE6) and it works fine. Amazingly, it even works on some ridiculously outdated version of Opera 8.5 that I had lying around my computer, for the 0.000001% of the population who might care... :)

The only thing I noticed is that the "This permission is inherited from the authenticated user role" title text is not showing up on hover, in Firefox at least (I forgot to check on the other browsers). Probably not that big of a deal.

However, I think I'm going to have some more comments about this patch come tomorrow.

Since it's committed and works on IE6, marking as fixed. Up to David whether you want to re-open or start new issues for followups.

I think this should be reopened because there were several problems brought up in the discussion above that were not addressed in the patch that was committed:

• As noted in #28 and #29, there is now an inconsistency between the information at admin/user/permissions and admin/user/permissions/X (where "X" is a role ID). A permission checkbox may be checked and disabled on the first page, but that exact same checkbox will be empty on the second page. The information on the two pages should be consistent.
• The patch doesn't make any visual distinction between implied permissions and permissions that are actually assigned. The only way you can really find out which is which is to untoggle the authenticated user checkbox and see which of the disabled checkboxes stick around and which don't. That's a bit unintuitive. Designs such as those in #34 were at least starting to move in the direction of trying to visually distinguish them.
• As noted in #23, this patch might make unassigning permissions difficult because it prevents people from clicking on the checkboxes they want to get rid of. Instead, they have to figure out that the only way to remove them is to unclick a separate checkbox way over on the left.

Also, I thought of another issue that was not raised previously:

• This patch introduces a particular interaction model on the permissions page, but that is not the only page in Drupal that presents a list of roles as checkboxes. Other places include (a) the role-specific visibility settings on blocks, (b) text format role assignments, etc., and the same considerations apply there. If a change is going to be made to one place, it seems like it should be made to all of them - otherwise people will be confused about why granting authenticated users the ability to do something sometimes disables the other roles and sometimes doesn't.

Overall, it seems to me that even if disabled checkboxes are considered a good idea in theory, the above problems shouldn't be ignored.

I just posted a patch which, as one of its side effects, would solve this problem in a totally different way:

#468768: Remove hardcoded anonymous and authenticated user roles

Basically this is the "nuclear option" :)

Should we make separate issues for the points in #57?

Point 1 about role specific permission pages, definately seems like a bug.

Issues for reworking the other role pages should probably be postponed on this issue until the unassigning method is worked out?

Still a valid issue after the release of 7.0, as per #57. Moving to 8.x, as this concerns the UX.

The functionality introduced by this patch causes significant performance issues on admin/people/permissions for large numbers of roles or permissions; see #1203766: With large number of permissions /admin/people/permissions becomes unusable.

yhea address them

Both Drupal 7 and 8 have this.