This patch does two things. First it fixes a bug introduced by the forms API that makes a theme show up on the user/x/edit form even if it is the only enabled theme. Second, it introduces a permission 'can choose theme'. A user cannot choose his or her own theme unless granted this permission. This is necessary as people are now creating multi themed sites using the sections module, and so forth. Many times I have two themes enabled for every site I run; one theme for production, and one for me to tinker with so that I can preview the site. I don't want my users to be able to choose my tinker theme as their default.

CommentFileSizeAuthor
#16 user_themes4.txt4.91 KBrobertdouglass
#15 theme_bugfix.txt3.4 KBrobertdouglass
#6 user_themes3.txt4.9 KBrobertdouglass
#1 user_themes2.txt4.88 KBrobertdouglass
user_themes.txt3.84 KBrobertdouglass

Comments

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new user_themes2.txt4.88 KB

Fixed some code style and added updated help text to user module.

saerdna’s picture

saerdna commented

errr i was about to follow up this but clumsy as i am sometimes i replied at my own. :-) I repost it here which it was supposed to.

i contributed with a simple fix yesterday ( http://drupal.org/node/38933 ) but this is better since it introduces a new access role.

+1

(would be cool if you could introduce a access role "allow username changes" too. I tried to look into it but im to new to drupal to make this proper)

saerdna’s picture

saerdna commented
Category: bug » task

im working some more on this patch, i submit later today. girlfriend wants to go on a xmas market now :-)

robertdouglass’s picture

robertdouglass commented

Glad you're working on it too. Make sure not to introduce too many issues in one patch, though. There are two already, which pushes the limit of acceptability. If you want to add other features or fix other problems, make sure to start new issues.

thanks!

-Robert

saerdna’s picture

saerdna commented

hmm im unable to apply the patch, it worked yesterday. crazy.

anyway i think its better to check if theres more than 1 theme (if(count($themes) > 1)) before theme stuff are made, should save some performence and memory. Instead of doing the themestuff and then unset it. that was the only change i tried to make. think you can fix it?

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new user_themes3.txt4.9 KB

heh, I always end up doing patches right before bed. Bad idea. Here's one that checks in the right place.

saerdna’s picture

saerdna commented

hmm what am i doing wrong? isnt patch -p1

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

No, it's patch -p0. Applied fine for me.

Overall I like, although as an earlier commenter said it's generally better to split a patch like this into two, one a bugfix and one a feature.

Code looks clean enough, functionality all works. My only negative comment is that "can choose theme" doesn't seem like a good name. It's much less formal sounding than the rest of the permissions in the core modules. Perhaps "may select own theme"? It's a minor niggle either way, so I'm going to set this ready to commit.

Of course, it would be even better if we could get a longer description of permissions in there, but that's been sitting in queue for a while now. </shameless plug>

+1.

saerdna’s picture

saerdna commented

great work!
+10 ;)

dries’s picture

dries commented

"can choose theme" sounds weird but I can't think of anything better though.

I wish this were two patches so that I could have fixed the bugfix.

Stefan Nagtegaal’s picture

Stefan Nagtegaal commented

How about 'access theme selection' or 'access theme selection page'?

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

I prefer the simplified version made by chx but I'm not 100% happy with the keys used in the links array.

  1. Some are really cryptic like blog_usernames_blog, comment_comments or comment_comments_new.
  2. Some keys are dynamic while other keys are not:
    +        $links['taxonomy_term_'. $tid]->title = $term->name;
+        $links['taxonomy_term_'. $tid]->href = "taxonomy/term/$term->tid";
Tobias Maier’s picture

Tobias Maier commented

or whats about
"change users' theme"
"user can select another theme"
"user can change (own?) theme"

Stefan Nagtegaal’s picture

Stefan Nagtegaal commented

Dries, I think you picked the wrong tab while updating the issues..
this seems related to the "Override restructured _links patch".

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new theme_bugfix.txt3.4 KB

Here's a bugfix patch. I'm not creating another issue for it.

robertdouglass’s picture

robertdouglass commented
StatusFileSize
new user_themes4.txt4.91 KB

How about "select personal theme" for the permission? This patch contains both the bugfix and the new permission.

dries’s picture

dries commented
Status: Needs work » Fixed

Committed to HEAD. Thanks.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)