If you create a maintenance page using Bartik (like making a minimal login page) and it uses d_s_m() somehow, you get this.

CommentFileSizeAuthor
#14 withPatchFirefox3.6.13.png39.58 KBTor Arne Thune
#14 withPatchChromium8.0.552.237.png34.17 KBTor Arne Thune
#13 1026654-bartik_maintanance_page_regression.patch864 bytesamateescu
#9 1026654-current-use.png35.09 KBDave Reid
#4 1026654-bartik-maintenance-messages.patch1.69 KBDave Reid
#4 1026654-after.png32.45 KBDave Reid
#3 1026654-bartik-maintenance-messages.patch1.02 KBDave Reid
#2 1026654-garland.png48.42 KBDave Reid
#1 1026654-before.png41.52 KBDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
1026654-before.png41.52 KB
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
1026654-garland.png48.42 KB

For reference, here's what the same looks like in Garland

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Active » Needs review
FileSize
1026654-bartik-maintenance-messages.patch1.02 KB

Initial patch to move messages to before content.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
1026654-after.png32.45 KB
1026654-bartik-maintenance-messages.patch1.69 KB

And with changes to messages CSS that appears to make everything better. Also attached screenshot of page with patch applied.

Status: Needs review » Needs work

The last submitted patch, 1026654-bartik-maintenance-messages.patch, failed testing.

Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented
Status: Needs work » Needs review

#4: 1026654-bartik-maintenance-messages.patch queued for re-testing.

jensimmons’s picture

jensimmons CreditAttribution: jensimmons commented

Well, the maintenance page was designed for displaying messages when the site is in maintenance mode, and when something really bad goes wrong — like when the database server goes down. Dave Reid is doing some very creative things using maintenance-page.tpl.php for other purposes (instead of making his own custom tpl file for the login page).

For reference, the theming of the maintenance page was worked out here: #790556: Make the Maintenance Page Kick Ass

The order was changed to make this:

screenshot of original maintenance page

look like this:

screenshot of current maintenance page

Displaying the error message after the "This website encountered an unexpected error" text is not a bug. That's as designed. If you look at the above screenshots, you'll see why that makes for better UX.

I don't know what to say about this super-edge case Dave-Reid special. My recommendation Dave, would be for you to duplicate the maintenance page and put that custom tpl file in your distribution, with the changes you desire.

Changing the width CSS from 700px to auto might be a good change — if it doesn't affect the typical usecase. I could see where it might help with people who are overriding things with Bartik in other cases.

But in general, this is as-designed.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

This is from a module, so I'm not sure how exactly I can achieve this. I don't understand how having messages below content is acceptable for bartik, but not for garland's maintenance theme. No where else do we output messages after output. :/

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
1026654-current-use.png35.09 KB

And this is how the maintenance page looks currently in CVS (aside from the width: auto fix which I left in). Is something going wrong that it looks so odd?

jensimmons’s picture

jensimmons CreditAttribution: jensimmons commented
Title: Messages on a Bartik maintenance page display below content » AAAAAA! - Bartik maintenance page regression bug
Status: Needs review » Needs work

Yeah that is messed. Up. :(
Something got changed, there should be no block box showing up. I bet when the last patch affecting the background color went in, there was no adjustment to the maintenance page styles to go with it.

#988026: Move background color from body to #page-wrapper

This needs to be fixed asap!!

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented

Subscribing. It really looks horrible to have the site in maintenance mode and by default have this page as the greeting to the visitor, with the message explaining that the site is under maintenance.

jensimmons’s picture

jensimmons CreditAttribution: jensimmons commented
Priority: Normal » Major

bumping

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Status: Needs work » Needs review
FileSize
1026654-bartik_maintanance_page_regression.patch864 bytes

Here is a patch that restores the maintaince page layout as in the second screenshot from #7.

Tested in FF 3.6, Chrome, IE8. In IE 6 and 7, there is no top margin for #page-wrapper, but I think that is out of scope for this issue.

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented
FileSize
withPatchChromium8.0.552.237.png34.17 KB
withPatchFirefox3.6.13.png39.58 KB

Ahh, much better. Tested on Firefox 3.6.13 and Chromium 8.0.552.237 on Linux.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

To be clear, this fixed a regression introduced in #988026: Move background color from body to #page-wrapper.
This is only a fix, not a cosmetic overhaul.

Tested, with screenshots, reviewed, ready to go.

jensimmons’s picture

jensimmons CreditAttribution: jensimmons commented
Priority: Major » Critical

I'm so glad we've got a fix ready, thanks Team Bartik!

Meanwhile, I'm going to bump this to critical. It's "only" a visual bug, but it's a really horrible one. Let's fix it immediately.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: AAAAAA! - Bartik maintenance page regression bug » Bartik maintenance page regression bug
Priority: Critical » Major

Please don't abuse the critical flag.

jensimmons’s picture

jensimmons CreditAttribution: jensimmons commented

another screenshot of the visual bug

This just kills me. :( Gets me right in the heart.

Can we please commit this fix? Dries?

webchick’s picture

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

Committed to HEAD.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Title: Bartik maintenance page regression bug » Bartik maintenance page places messages below content
Status: Fixed » Active

I would still like to re-consider #4.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Active » Needs review
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Category: bug » task
Priority: Major » Normal
jacquesboucar’s picture

jacquesboucar CreditAttribution: jacquesboucar commented

I tested patch on 7.11-dev but here is failed at 5, 24 and 58

  • webchick committed a4af614 on 8.3.x 
    #1026654 by amateescu: Fix regression with background colours in Bartik...

  • webchick committed a4af614 on 8.3.x 
    #1026654 by amateescu: Fix regression with background colours in Bartik...

  • webchick committed a4af614 on 8.4.x 
    #1026654 by amateescu: Fix regression with background colours in Bartik...

  • webchick committed a4af614 on 8.4.x 
    #1026654 by amateescu: Fix regression with background colours in Bartik...

  • webchick committed a4af614 on 9.1.x 
    #1026654 by amateescu: Fix regression with background colours in Bartik...