| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
I realize I suggested this grand hack late in the Drupal release cycle at http://drupal.org/node/192779#comment-661039 and committed these special cases in #192779: Don't show disabled themes on blocks settings page and #201577: show admin theme on page admin/build/themes/settings, but this was one very bad idea.
The original issue as I understand it is that if the admin theme is *enabled*, it can be selected by those users who have the permission to select themes. And this is not desired. So the D6 "solution" was to let the admin theme "work" while being disabled (so you can configure blocks and its settings even though it is disabled). So what's an enabled theme really boiled down to who you ask. If the end user, it was the themes you've set enabled. If Drupal's blocks and themes subsystem, it was the enabled themes plus the admin theme. But what is dangerous is that other subsystems of Drupal like update_status do not have this special casing for admin themes, and when disabled, they are just considered disabled. So if you use a contributed admin theme, update status will never tell you if you have a security update to it, your site will keep being vulnerable (and having your *admin* theme vulnerable can be quite an issue).
So we can continue sprinkling Drupal with special casing of disabled admin themes (more hacks), but in reality we should just fix our mental model. We should raise the question: what does enabling a theme means?
- You can configure its own settings
- You can configure blocks for it
- You can pick it as an admin theme
- You get update status reports about it
Drupal 6 also used to say "- Your users (with appropriate permissions) can pick this theme", but due to #292253: Remove the per-user themes selection from core this is not present in Drupal 7 anymore. However, if we'd consider user theme selection in core, the solution is still not to let the admin theme be disabled, since all the above and other theme related features need ugly special casing. Instead, we should fix the user theme selector then to not include the admin theme. Or even better, just have a configuration screen to pick which themes are available to users, so the above list would be extended with:
- You can pick the theme as one of the user selectable themes
This would map to sites where modules like themekey are used to switch themes based on sections. Maybe not all of those themes would the site owner expose to users. While we can exclude the admin theme, the more generic use case of themekey + user specific theming is impossible to infer programatically.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| no-more-disabled-admin-theme.patch | 2.94 KB | Idle | Failed: 11822 passes, 3 fails, 0 exceptions | View details | Re-test |
Comments
#1
I'm working on implementing #491214: implement the top level Appearance / Choose Theme admin page, and we really need a definition of what enabled means, so therefore the overall thinking on its issues and the attempt to properly solve them finally. In other words, fixing this would be a good kick to the Appearance page in the D7UX implementation.
#2
The last submitted patch failed testing.
#3
subscribe
#4
+
#5
This is not an issue. A user should have the right to use Seven as the default theme. Even if Seven does not have all the regions as one might expect, it still is a good theme for regular pages. :-)
#6
@xmacinfo: ok, I'll try to be more clear. The original issue **in Drupal 6** was that there was a feature called user theme selection where all enabled themes showed up without consideration. So therefore if you wanted to offer theme selection to users, but exclude the admin theme, you'd need to disable your admin theme so it does not show up in that list. Therefore the hacks which IMHO should be removed now that we have no user theme selection. If we are going to have user theme selection or some other similar feature, we should have a selection of the themes supported on the UI with a user interface to pick which themes to list, so hacks like disabling some themes will not be required.
#7
@Gábor: Those theming issues can become quite complex and hard to follow. Yes, now the end user cannot change theme. In my message I should have written the Admin user can use any theme and mark it as admin, whereas any theme, including Seven, can be used as a regular theme. So, yes, we must not special case the admin theme, disabled or not. Forgive me if I miss some points. :-)
Can you reroll this patch?
#8
subscribing, and reading a couple more times
#9
reroll, with changes:
1. made sure that the admin theme is enabled after submitting the theme settings form
2. the wrong block test removed
3. one wrong and useless message removed
#10
The last submitted patch failed testing.
#11
rerolled without the useless message, see: #556714: Remove the administration theme status message
#12
Looks great. I'd only add some parenthesis in the .. || .. == .. || .. == .. sequence to make it more clear :)
#13
added parenthesis
#14
The last submitted patch failed testing.
#15
retest?
#16
The last submitted patch failed testing.
#17
reroll
#18
The last submitted patch failed testing.