In #1360994: Change hook_maintenance_page to hook_maintenance it was observed that maintenance-page.tpl.php confused themers working on the html5 initiative, because they expected it to be similar to page.tpl.php, and yet it wasn't. That issue initially suggested renaming the theme hook to simply 'maintenance', but in this issue, rather than doing that, I'd like to propose making them consistent. This patch:

  • Changes maintenance-page.tpl.php to be wrapped by html.tpl.php, as page.tpl.php is, rather than having to inline that markup.
  • Changes region content within maintenance-page.tpl.php to render arrays within $page rather than top-level strings. Note that the documentation at the top of maintenance-page.tpl.php already says that variables are mirrors of page.tpl.php, but that's currently a lie, so this makes that documentation true.
  • Removes a bunch of duplication from the preprocess/process functions as can be seen by all the minus signs in the patch. This will also make it easier to keep this consistent in the future.
  • Removes the 'install_page' and 'update_page' theme hooks. Those look like useless wrappers. Makes them template suggestions instead (e.g., maintenance-page--install, etc.), and extends that to other maintenance modes including 'error', 'site_offline', and 'batch'.
  • Renames the 'maintenance-page--offline' suggestion to 'maintenance-page--db-offline' to clarify and distinguish from 'maintenance-page--site-offline'.
  • Adds the same suggestion pattern to 'html' for the different maintenance modes (e.g., html--maintenance, html--maintenance--install, html--maintenance--site-offline). There's no implementation of any of these in core, because for now, html.tpl.php suffices, but I added the pattern for consistency, and who knows, maybe someone will find a use for it.
  • Removes the 'maintenance-page' and 'in-maintenance' CSS classes for the <body> tag (Bartik used one and Seven used the other), and replaces with 'maintenance-mode'. I'm open to reverting that to one of the other two (but just one, please) if one of those names is preferred.

 
From #1360994-20: Change hook_maintenance_page to hook_maintenance:

It would be great if we could actually use page__maintenance.tpl.php, but the preprocess functions for page contain too many things that depend on a database.

Yeah, that's why this issue keeps it as its own primary theme hook ('maintenance_page') instead of making it a suggestion only ('page__maintenance'). This patch has template_preprocess_maintenance_page() call template_preprocess_page() to minimize code duplication, and therefore changes template_preprocess_page() to not do things it shouldn't do in maintenance mode, but otherwise, the preprocess pipeline is kept separate, so that module and theme implementations of hook_preprocess_page() don't have to worry about maintenance mode.

But the *_preprocess_html() pipeline is the same for regular and maintenance pages, so any module/theme implementing those functions needs to check $variables['page']['#maintenance mode'] if doing something that shouldn't be done in maintenance mode. I think that's ok, because 'html' is intended as a reusable, generic template, so modules and themes *shouldn't* be doing anything crazy in those preprocess functions.

CommentFileSizeAuthor
#11 drupal-1472542-11.patch32.36 KBtim.plunkett
#8 theme.maintenance_page.8.patch32.55 KBeffulgentsia
#7 theme.maintenance_page.7.patch28.52 KBeffulgentsia
#4 theme.maintenance_page.4.patch28.94 KBeffulgentsia
#2 theme.maintenance_page.2.patch25.26 KBeffulgentsia
theme.maintenance_page.patch24.64 KBeffulgentsia

Comments

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

Status: Needs review » Needs work

The last submitted patch, theme.maintenance_page.patch, failed testing.

effulgentsia’s picture

effulgentsia commented
Status: Needs work » Needs review
StatusFileSize
new theme.maintenance_page.2.patch25.26 KB

Status: Needs review » Needs work

The last submitted patch, theme.maintenance_page.2.patch, failed testing.

effulgentsia’s picture

effulgentsia commented
Status: Needs work » Needs review
StatusFileSize
new theme.maintenance_page.4.patch28.94 KB
effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

kika’s picture

kika commented

Love those ---

git diff --stat
 core/authorize.php                                 |    2 +-
 core/includes/batch.inc                            |    2 +-
 core/includes/common.inc                           |   75 ++++++++--
 core/includes/errors.inc                           |   11 +-
 core/includes/install.core.inc                     |    2 +-
 core/includes/theme.inc                            |  158 +++++---------------
 core/includes/theme.maintenance.inc                |   59 ++++----
 core/modules/overlay/overlay.module                |   12 --
 core/modules/system/maintenance-page.tpl.php       |   32 +---
 core/modules/system/system.module                  |    5 +
 .../bartik/templates/maintenance-page.tpl.php      |   19 +---
 core/themes/seven/maintenance-page.tpl.php         |   27 +---
 core/themes/seven/template.php                     |   12 --
 core/update.php                                    |    4 +-
 14 files changed, 159 insertions(+), 261 deletions(-)

Status: Needs review » Needs work

The last submitted patch, theme.maintenance_page.4.patch, failed testing.

effulgentsia’s picture

effulgentsia commented
Status: Needs work » Needs review
StatusFileSize
new theme.maintenance_page.7.patch28.52 KB
effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
StatusFileSize
new theme.maintenance_page.8.patch32.55 KB

Oops. Forgot the change in CSS files. This has them, and I updated the issue summary to reflect the current state of the patch.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

effulgentsia’s picture

effulgentsia commented
Issue summary: View changes

Updated issue summary.

Niklas Fiekas’s picture

Niklas Fiekas commented
Status: Needs work » Needs review
Issue tags: -html5, -consistency

Indeed --++. Thank you!

The patch no longer applies and can't be auto-merged. So this needs a reroll.

#8: theme.maintenance_page.8.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, theme.maintenance_page.8.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: +html5, +consistency
StatusFileSize
new drupal-1472542-11.patch32.36 KB

drupal_deliver_html_page is not called directly in core, and I'm pretty sure it is being replaced by Symfony Response stuff.

However, I just wanted to get this patch applying as close to its original form as possible, so no other changes.

The last submitted patch, drupal-1472542-11.patch, failed testing.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Closed (duplicate)
Related issues: +#2218039: Render the maintenance/install page like any other HTML page, +#2231853: Rename maintenance-page template into page--maintenance + install-page into page--maintenance--install

Oops. This issue did not appear in my search results back then. Anyway, this is resolved now. (the final patch looked similar)