drupal_common_theme() should explicitly add the file include/theme.maintenance.inc when registering 'maintenance_page', because template_preprocess_maintenance_page() is in that (conditionally included) file.

The test for this is: apply #243532: Catch notices, warnings, errors and fatal errors from the tested side, run the "Simpletest functionality" test, and see the beautiful notices from the tested site.

CommentFileSizeAuthor
#16 321828-maintenance-page-4.patch9.91 KBdavyvdb
#13 321828-maintenance-page-3.patch10.89 KBdavyvdb
#11 321828-maintenance-page-2.patch10.49 KBdavyvdb
#10 321828-maintenance-page-2.tpl_.php.txt10.49 KBdavyvdb
#8 321828-maintenance-page.tpl_.php.txt5.43 KBdavyvdb
maintenance-page-register.patch752 bytesdamien tournoud

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Fixed

Fix looks good, but we don't define arrays that way. :) (Except for 'arguments' which I guess is to make it look more like a function signature).

Changed the patch to put each key on its own line and committed. Thanks!

Status: Fixed » Closed (fixed)

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

merlinofchaos’s picture

merlinofchaos commented
Status: Closed (fixed) » Active

This is broken. Right now the theme system does not accept having a template and a specified file containing a preprocess function in different directories. The 'path' is used for both.

merlinofchaos’s picture

merlinofchaos commented

There are 3 possible solutions:

1) move theme.maintenance.inc into system as system.maintenance.inc
2) move the template to includes
3) split 'path' so that we can have a template path and a file path.

Solution 2 is clearly the weakest.

Solution 1 is probably the easiest.
Solution 3 could solve a few other issues, but I think would lead to a larger re-org, because we would almost certainly move all core templates into one directory (which would still leave module's templates out in the cold but what can you do?)

johnalbin’s picture

johnalbin
they/他
Primary language English
Location Taipei, Taiwan
commented

BTW, if you want to see the bug, set the Stark theme as your maintenance theme in settings.php. You'll get a blank page because it tries to look in "includes" for the maintenance-page.tpl.php file.

davyvdb’s picture

davyvdb commented
Priority: Normal » Critical
davyvdb’s picture

davyvdb commented

This extremely easy patch that sets the admin theme on /batch fails because of this http://drupal.org/node/563634

The maintenance-page is in system.module and the theme function is in theme.inc. How can we solve this?

davyvdb’s picture

davyvdb commented
Status: Active » Needs review
StatusFileSize
new 321828-maintenance-page.tpl_.php.txt5.43 KB

This seems to solve this on my local machine.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Needs work

Well this patch simply remove the maintenance page theme ;)

davyvdb’s picture

davyvdb commented
Status: Needs work » Needs review
StatusFileSize
new 321828-maintenance-page-2.tpl_.php.txt10.49 KB

Wrong patch. That was weird.

davyvdb’s picture

davyvdb commented
StatusFileSize
new 321828-maintenance-page-2.patch10.49 KB

Of course a patch extension is needed. Ouch.

Status: Needs review » Needs work

The last submitted patch failed testing.

davyvdb’s picture

davyvdb commented
StatusFileSize
new 321828-maintenance-page-3.patch10.89 KB
davyvdb’s picture

davyvdb commented
Status: Needs work » Needs review
pasqualle’s picture

pasqualle
Location 🇭🇺 Budapest
commented 
+  arg(0) == 'batch'

this change should not be in the patch

+
+
+

please use only 1 empty line between functions

davyvdb’s picture

davyvdb commented
StatusFileSize
new 321828-maintenance-page-4.patch9.91 KB
roychri’s picture

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

This fixes the problem for me.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD! Thanks! I will be so happy for those phantom errors about this file being missing to go away. :)

Status: Fixed » Closed (fixed)

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