693592 - Footer breaks on captcha page

Status: RESOLVED FIXED

(mozilla.org Graveyard :: Webdev, task)

Description

[Schalk Neethling [:espressive]]

When submitting a comment on the WebDev blog, one is first redirected to the simple captcha page. On this page the footer of the blog 'breaks' i.e. it does not stretch to fit the window and is not aligned at the bottom.

Comment 1

[Schalk Neethling [:espressive]]

This is caused by the way the simple captcha page is built. The footer get's pulled into the body div. We need a slight rewrite of the view companent of simple captcha. 

This will most likely mean that we have to create a clone of the current one and call it webdev-simplecaptcha

Comment 2

[Schalk Neethling [:espressive]]

Actually, we might be able to build in an if/else check. So, if it is the WebDev blog, load a custom header and if not, just include the default get_header() call as it does now.

Comment 3

[Schalk Neethling [:espressive]]

Attachment: Fixed captcha page

This fix involves edits in two files and the creation of one new one.

Edits:
\wp-content\themes\unstunst\css\master.css
\wp-content\plugins\simplecaptcha\simplecaptcha_plugin.php

New file:
\wp-content\themes\WebDevTheme\header-captcha.php

Comment 4

[Fred Wenzel [:wenzel]]

(In reply to Schalk Neethling from comment #3)
> This fix involves edits in two files and the creation of one new one.

Care to share the code :)

Comment 5

[Schalk Neethling [:espressive]]

Attachment: New custom header fot the captcha page

Comment 6

[Schalk Neethling [:espressive]]

This is all in Subversion, not sure if I have commit access yet, so inlining it hear,hope that is ok. I will attach the new header file though.

master.css

#archive #headwrap,
#captcha  #headwrap{
    height: 60px;
}

#archive #logo,
#captcha #logo {
    width: 783px;
    height: 60px;
    text-align: left;
}

#archive #logo h1 a,
#captcha  #logo h1 a {
    height: 60px;
    font-size: 2em;
    line-height: 1;
    letter-spacing: 0;
    position: absolute;
    left: 40px;
    bottom: -23px;
    text-transform: uppercase;
	-mozilla-transform: rotate(0deg);
	-webkit-transform: rotate(0deg);
	color: #fff;
}

#archive #header .sidebar,
#captcha #header .sidebar {
    height: 60px;
    padding: 0 0 0 15px;
    border: 0;
}

#archive #header .sidebar ul,
#captcha #header .sidebar ul {
    width: 190px;
}

#archive #header .sidebar li,
#captcha #header .sidebar li {
    display: inline;
    font-size: 0.85em;
    padding-right: 15px;
}

#archive #header .sidebar ul li:before,
#captcha #header .sidebar ul li:before {
    color: rgb(255,255,255);
    content: "";
}
#captcha #footerwrap {
	position: fixed;
	bottom: 0;
	padding-bottom: 0;
    font-size: 1.7em;
}

------

Changed from:

get_header()

to

<?php
	if(strrpos($_SERVER['REQUEST_URI'], 'webdev')) {
		get_header('captcha');
?>

<div id="body">
	<div id="blogwrap">
		<div id="blog">

<?php
	} else {
		get_header();
	}
?>

and fron

get_footer()

to

<?php
	if(strrpos($_SERVER['REQUEST_URI'], 'wp-content')) {
?>
		</div>
	</div>
</div>
<?php
		get_footer();
	} else {
		get_footer();
	}
?>

Comment 7

[Schalk Neethling [:espressive]]

Oops typo on the footer section:

Needs to be:

<?php
	if(strrpos($_SERVER['REQUEST_URI'], 'webdev')) {
?>

Comment 8

[Fred Wenzel [:wenzel]]

Hm, did you check out the code from SVN? Then you can just do ``svn diff`` and it'll give you a diff. Put that into a file (``svn diff > mypatch.patch``). Then attach it to this bug and put someone into "review ?".

Comment 9

[Schalk Neethling [:espressive]]

Attachment: Patch for this bug

Comment 10

[Schalk Neethling [:espressive]]

[:wenzel] Ok, added the patch file. Who would be the person I should set to review?

Comment 11

[Schalk Neethling [:espressive]]

Attachment: Revised Patch for this bug

Comment 12

[Fred Wenzel [:wenzel]]

Comment on attachment 566271 [details]
Revised Patch for this bug

Hm, this is a global plugin, I am not comfortable with adding special case for webdev there. Any way you can do it without messing with the plugin?

Comment 13

[Schalk Neethling [:espressive]]

[:wenzel] Nope, the way the theme header is built and the way the plugin pulls in the header and footer, causes the footer to be enclosed in the blog body. The body has a set width and the footer, being set to 100%, now only takes up the width set by the body container.

With this change, I ensure that the body container(s) are closed before loading the footer and now, the footer can extend to the full width of the browser window. I have one option that I am going to explore now that might, fingers crossed, just work. If not, then the current patch seems to be the only way around that unless of course, we do not use the plugin ;)

Comment 14

[Schalk Neethling [:espressive]]

I believe I have found a way, stay tuned....

Comment 15

[Schalk Neethling [:espressive]]

What if I can lessen the condition to just this?

<?php 
	if(strrpos($_SERVER['REQUEST_URI'], 'wp-content')) {
		get_header('captcha');
	} else {
		get_header();
	}
?>

Comment 16

[Schalk Neethling [:espressive]]

ignore wp-content this should be webdev

Comment 17

[Schalk Neethling [:espressive]]

Attachment: Revision 2 Patch for this bug

Comment 18

[Schalk Neethling [:espressive]]

[:wenzel] - Just thought of a way I can fix this without touching the plugin. Will send new patch.

Comment 19

[Schalk Neethling [:espressive]]

Attachment: Revision 3 Patch for this bug

Comment 20

[Schalk Neethling [:espressive]]

Hey [:wenzel], have you had a chance to review the latest patch?

Comment 21

[Fred Wenzel [:wenzel]]

Comment on attachment 566309 [details] [diff] [review]
Revision 3 Patch for this bug

Review of attachment 566309 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

r+ with comments

::: css/master.css
@@ +745,5 @@
>      font-size: 1em;
>  }
>  
> +#captcha #footerwrap {
> +	position: absolute;

Fix indentation in this block: Four spaces, not tabs. In fact, never use tabs.

::: header.php
@@ +20,5 @@
>  <?php wp_head(); ?>
>  </head>
>  
> +<?php
> +if(strrpos($_SERVER['REQUEST_URI'], 'simplecaptcha')) {

nit-pick: if is a language construct, not a function, so there's a space after it.

Comment 22

[Schalk Neethling [:espressive]]

Thanks for the pointers [:wenzel] will definitely keep them in mind.

Comment 23

[Schalk Neethling [:espressive]]

Not sure if I have SVN commit access yet but, if I do, should I commit these?

Comment 24

[Fred Wenzel [:wenzel]]

(In reply to Schalk Neethling from comment #23)
> Not sure if I have SVN commit access yet but, if I do, should I commit these?

Yes! If you do not have commit access, work with Laura to get it. Here's a hint:
http://www.mozilla.org/hacking/committer/

Comment 25

[Schalk Neethling [:espressive]]

Hey [:wenzel],

Still awaiting SVN access ;(

https://bugzilla.mozilla.org/show_bug.cgi?id=689756

Comment 26

[Schalk Neethling [:espressive]]

[:wenzel] Changes are checked in

Comment 27

[Fred Wenzel [:wenzel]]

Cool, thanks. It's good practice to:
- mention the bug number in the commit text ("added a crocodile to the header. bug 693592.")
and
- mention the SVN revision or github commit link in the bug. r1234 is auto-linked. ("fixed in r12345")

Comment 28

[Schalk Neethling [:espressive]]

[:wenzel] Thanks for the info, will make sure to include these in future. I am using Toortoise SVN, will have to learn about geting that info out of it.