path_is_admin() function should provide a list of administrative path. It does not.

It is currently only used in any meaningful way by system_custom_theme() to change the theme when "Use the administration theme when editing or creating content" is checked.

The patch in parent issue wants to use path_is_admin() but can't rely on it. whatever happens, node/*/edit is an administrative page. If it's just about the theme, there should be another way to deal with that than lie about admin paths.

Problem


/**
 * Implements hook_custom_theme().
 */
function system_custom_theme() {
  if (drupal_container()->isScopeActive('request')) {
    $request = \Drupal::request();
    $path = $request->attributes->get('_system_path');
    if (user_access('view the administration theme') && path_is_admin($path)) {
      return \Drupal::config('system.theme')->get('admin');
    }
  }
}

It is not the responsibility of path_is_admin() to exclude paths where the frontend theme should be used for administrative tasks.

Comments

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented

The patch in parent issue wants to use path_is_admin() but can't rely on it. whatever happens, node/*/edit is an administrative page.

I don't agree really. When you created this issue on drupal.org (or if I were to edit the issue summary) would we be performing "administrative tasks"? It definitely doesn't feel like an administrative task to me.

I think it's correct for node/*/edit to be conditionally included/excluded based on how the site is configured, but the issue might be that the configuration is misnamed given what it actually does (i.e., the setting the Node module uses for all this is called \Drupal::config('node.settings')->get('use_admin_theme') and described as being only about the theme in the user interface also).

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

So it turns out it's not a blocker for the overlayslayer patch, it's still confusing though.

Maybe the the tagged link issue might be a way to solve this?

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Active » Closed (won't fix)

It's a non issue. path_is_admin() has always been used like that. Let sleeping dogs lie.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Issue tags: +Drupal wtf

#2264607: path_is_admin() lies: it considers node/*/edit not an admin route when using Bartik for node/*/edit ran into this also. It's a major WTF, IMHO.

But fortunately this has been fixed in 8 now AFAICT. Fixing this in 7 would be an API change, so won't happen, sadly.

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein as a volunteer commented
Related issues: +#2934255: Node module's "use_admin_theme" setting is misnamed, since it's about more than just the theme

I don't see how this is fixed in Drupal 8 - it seems to be the same behavior. See https://api.drupal.org/api/drupal/core%21modules%21node%21src%21EventSub... and https://api.drupal.org/api/drupal/core%21modules%21node%21node.routing.y....

Per above, I still think "won't fix" is the correct status, though.

I went ahead and filed an issue for my suggestion above (to rename the Node module setting so it doesn't claim to only be about the administrative theme).