CustomError 7.x-1.2 is causing the default theme to be used on certain administration pages (in non-error situations entirely) instead of the admin theme.

For me this is happening on Features' Review Overrides pages (/admin/structure/features/[featurename]/diff) and upon saving views.

customerror_custom_theme() (added in 7.x-1.2 in commit #6b3f920) is triggering a call to menu_get_item() via drupal_valid_path(). When menu_get_item() is called before Drupal's main bootstrap process like this, it can initialize the theme system too early.

This has happened in other modules. See:

Looks to be a core bug, ultimately, and this whole subsystem has been redone in D8: #1954892: Replace 'theme callback' and hook_custom_theme() with a clean theme negotiation system.

Is there a way we can get around it in D7?
Some way we can avoid calling get_menu_item() from hook_custom_theme()?

In that same commit to CustomError (#6b3f920) a theme callback was added:

  $items['customerror'] = array(
    'title' => 'customerror',
    'access callback' => TRUE,
    'page callback' => 'customerror_page',
    'theme callback' => 'customerror_theme_callback',
    'type' => MENU_CALLBACK,
  );

The customerror_theme_callback() callback function is not defined in the module, though. I tried defining it and, inside it, doing essentially what hook_custom_theme() does, but CustomError's theme callback does not get triggered on error pages; only when loading the URL customerror/[errcode] directly.

CommentFileSizeAuthor
#3 customerror-hook_custom_theme_order-2347825.patch513 bytesjeffschuler

Comments

jeffschuler’s picture

jeffschuler
Location Boulder, Colorado
commented
Issue summary: View changes
gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Status: Active » Closed (duplicate)
Parent issue: » #278575: Admin theme is used on error pages below /admin

Looks to be a core bug, ultimately, and this whole subsystem has been redone in D8: #1954892: Replace 'theme callback' and hook_custom_theme() with a clean theme negotiation system.
Is there a way we can get around it in D7?

Not that I am aware of.

jeffschuler’s picture

jeffschuler
Location Boulder, Colorado
commented
Version: 7.x-1.2 » 7.x-1.x-dev
Status: Closed (duplicate) » Needs review
StatusFileSize
new customerror-hook_custom_theme_order-2347825.patch513 bytes

Thanks for the quick response, @gisle.
I don't believe this issue to be a duplicate of #278575: Admin theme is used on error pages below /admin; the issues describe quite different resulting problems.

To help minimize some of the trouble caused, can we change the order of conditionals in customerror_custom_theme() so the problem described in this issue won't occur if there is no custom theme set for CustomError?

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented

OK, I'll try to find some time to look at it.

The addition of customerror_theme_callback() in the commit you refer to was is the remains of a (failed) attempt to fix the bug you say this is not a duplicate of.

(The presence of a theme_callback defintion without a callback function in 7.x-1.2 is confusing and it should have been removed after it was determined that it did not have the desired effect. I'll make sure it is gone in the next snapshot.)

jeffschuler’s picture

jeffschuler
Location Boulder, Colorado
commented

Thanks.

Just for reference, it looks like someone else has had issues with theme_callback and error pages: How can can I get theme callback to work for an error page? | Drupal Answers. No good answers there, either, (including the suggestion to use menu_get_item() in hook_custom_theme()).

  • gisle committed d87b187 on 7.x-1.x authored by jeffschuler 
    Issue #2347825 by Jeff Schuler: Changed order of conditionals to...
gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Status: Needs review » Reviewed & tested by the community

This has been committed to the latest snapshot release of 7.x-1.x-dev, and will be included in the next regular release.

sgdev’s picture

sgdev commented

Thanks for making this patch, much appreciated!

A quick comment for anyone planning to use the patch in #3 -- there seems to be a formatting issue with the file. Just add the change manually and it will take care of the problem.

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented

@ron_s wrote:

A quick comment for anyone planning to use the patch in #3 -- there seems to be a formatting issue with the file. Just add the change manually and it will take care of the problem.

Or you may download the lastest snapshot of the 7.x-1.x branch from the repo. The patch has been included.

sgdev’s picture

sgdev commented

Just to confirm... did you mean 3.x or 1.x?

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented

Sorry. I meant 7.x-1.x (there is no 7.x-3.x). Corrected.

cthshabel’s picture

cthshabel commented

WOW... that was concerning. we have recently done a lot of different development changes and this we noticed.

thanks a lot for the patch and the keywords in this queue. I was able to quickly find this and things are working again with the patch in #3.

Thanks!

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
commented
Version: 7.x-1.x-dev » 7.x-1.3
Status: Reviewed & tested by the community » Fixed

I am setting this to fixed in release 7.x-1.3. It probably cannot be fully resolved until D8, tho' - see comment #2.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.