Discovered by #402254: How do we do on XHTML validation?: custom 404 pages are displayed mirrored, one into the other:

CommentFileSizeAuthor
#7 405086-error-pages-displayed-twice.patch1.28 KBdamien tournoud
#6 405086-error-pages-displayed-twice.patch1.28 KBdamien tournoud
#4 405086-error-pages-displayed-twice-test.patch1.56 KBdamien tournoud
#1 405086-custom-404-weirdness.patch1.37 KBdamien tournoud

Comments

damien tournoud’s picture

damien tournoud commented
Status: Active » Needs review
StatusFileSize
new 405086-custom-404-weirdness.patch1.37 KB

And a patch for review.

brianV’s picture

brianV commented

Tested patch, applies cleanly and fixes the nested 404s. I'll let some others monkey around with it, but looks RTBC to me.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Custom 404 pages weirdness » Custom 404 pages displayed twice
Status: Needs review » Needs work

Fixing title.

Also, since I've seen this bug occur intermittently due to a variety of factors, we should add a test for it. Looks like #402804: New Assertion: Check for test being found only once could be an enabler for that.

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Needs review
StatusFileSize
new 405086-error-pages-displayed-twice-test.patch1.56 KB

This is a test for the bug, it is supposed to fail.

Status: Needs review » Needs work

The last submitted patch failed testing.

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Needs review
StatusFileSize
new 405086-error-pages-displayed-twice.patch1.28 KB

Here is a patch.

After discussing with Angie, I don't believe this requires a specific test. We don't test this explicitly everywhere else. Our goal should be to turn on XHTML validation (#402254: How do we do on XHTML validation?), as soon as we will do, that type of error will be caught anyway.

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 405086-error-pages-displayed-twice.patch1.28 KB

Reupload for the test bot.

brianV’s picture

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

Looks good to me

webchick’s picture

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

Hm. Do you see this error anymore in HEAD? For some reason I don't.

I went under site information and set my custom 404 to node/1, and when I went to jhsdkhjskhfksd I got node/1 back only once.

Status: Needs review » Needs work

The last submitted patch failed testing.

Tor Arne Thune’s picture

Tor Arne Thune commented
Status: Needs work » Closed (fixed)

This is not a problem anymore in 7.0. Closing as fixed.