Currently when the admin pages are visited by anonymous users the admin theme is displayed. I would think this should not be the case as it makes the site look inconsistent to a user who might just have accidentally visited an incorrect URL. To get back to the rest of the site, users now have to fumble around a completely new interface to find any links. I know this has been jarring to me on occasion, even though I am used to the admin theme I setup.
The change would be for user access to be determined before a theme is switched.
I'm classifying this as a bug report, as I think this hurts usability of the site.
Comment | File | Size | Author |
---|---|---|---|
#4 | admin_theme.patch | 1.02 KB | Susurrus |
#1 | default_theme_on_admin_paths_7.x.dev_.patch | 1.02 KB | gdevlugt |
#1 | default_theme_on_admin_paths2_7.x.dev_.patch | 1.08 KB | gdevlugt |
Comments
Comment #1
gdevlugt CreditAttribution: gdevlugt commentedI added two different patches for Drupal 7.x dev which both will set the theme for access denied pages to either the custom theme of the user, or the sites default theme. While both do the same, the first one fixes the issue by checking in init_theme() whether or not a 403 header is present. The second one solves things in drupal_access_denied() by determining which theme to use.
I'm not sure which one is the best approach. Personally I feel more for the first one since it doesn't duplicate code from init_theme, introduce globals into drupal_access_denied() and simply is smaller.
Comment #2
dvessel CreditAttribution: dvessel commentedThis should be done in system_init where the admin theme is initially picked out.
Comment #3
Susurrus CreditAttribution: Susurrus commentedWould it not make more sense to do a
strpos(drupal_get_headers(), '403 Forbidden')
instead? I leave out the HTTP/1.1 designator as it's not really needed and may be a little too specific...Comment #4
Susurrus CreditAttribution: Susurrus commentedJust added my suggestions in #3 to 1st patch provided in #1.
Comment #5
dvessel CreditAttribution: dvessel commentedOo, you know, drupal_access_denied actually looks good as long as it's called before system_init. Haven't tested.
Comment #6
dvessel CreditAttribution: dvessel commentedSusurrus, I don't think that's a good place to do it. Other modules can set $custom_theme and you're forcing a specific behavior for 403's. Didn't look closely enough to be sure but $custom_theme should be set before init_theme I think.
Comment #7
Susurrus CreditAttribution: Susurrus commentedI'm not sure where the best place to put this code is, as I'm not familiar with this part of core. I just made a small change to the code that checks for a 403 error and just submitted the patch that you initially suggested. So which one should we go with?
Comment #8
gdevlugt CreditAttribution: gdevlugt commentedThe patch, after reading the original issue again and the new comments, I think would need to do the following :
The patches I provided however were, concerning point 1 a bit less restrictive since it would switch back on every user access denied 403. Whether or not this is good depends on contrib modules setting a custom theme. In some cases you don't want the custom theme to show, but in others perhaps you do want it. What it should be is up to others (not me) to decide.
If you only want it in the admin pages, ideally, system_init like dvessel suggested would be the place since this is the place where the administrative theme gets set. However, at that point it's unclear if the user has access or not. A check for 403 won't work there since it gets set later. Also, a check through user_access isn't an option since there are a lot of core admin permissions, and any contrib module can add admin permissions which also needs to be checked. Unless there's a function which can check if the user has access to a certain path, I'm not sure if its worth checking each permission just to switch a theme back.
I would be happy to provide a patch, but first I'd like some more input on what approach should be taken.
Comment #9
dvessel CreditAttribution: dvessel commentedI'm not certain but isn't there a menu function for checking access permissions? If so, then it can be done with that. Wouldn't need to check for 403's specifically in system_init. Chx would know this.
Comment #10
vladimir.dolgopolov CreditAttribution: vladimir.dolgopolov commentedThis is the test for the issue.
Patches #1 pass it.
Patch #4 fails it.
Concerning 403 check this out: Custom 403 page not available to user who doesn't have 'access content' permissions
Comment #11
Robin Monks CreditAttribution: Robin Monks commentedBased on comments above.
Robin
Comment #12
open-keywords CreditAttribution: open-keywords commentedPlease also consider this discussion http://drupal.org/node/242200 (edit operations and trouble in block module )
Comment #13
effulgentsia CreditAttribution: effulgentsia commentedIf I understand this issue correctly, then I believe it's fixed, in that if an anonymous user goes to /admin, the access denied message is displayed using the default theme, not the admin theme.
Comment #15
davidwhthomas CreditAttribution: davidwhthomas commentedHere's a small module snippet to force the default theme on admin pages to anonymous users:
Just a workaround for those looking for an immediate solution.
DT
Comment #16
edgartanaka CreditAttribution: edgartanaka commentedIn Drupal 6, my solution was:
- create a page for access denied with PHP formatting type. You want to add the following piece of code in the beggining of the body:
- set the URL to the 403 page you created in /admin/settings/error-reporting
Comment #17
edgartanaka CreditAttribution: edgartanaka commentedMy solution was:
- create a page for access denied with PHP formatting type. You want to add the following piece of code in the beggining of the body:
- set the URL to the 403 page you created in /admin/settings/error-reporting