Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#36510 closed defect (bug) (fixed)

Twenty eleven page templates with widgets incorrectly styled

Reported by: clorith's profile Clorith Owned by: swissspidy's profile swissspidy
Milestone: 4.5.1 Priority: normal
Severity: normal Version: 4.5
Component: Bundled Theme Keywords: has-patch commit fixed-major
Focuses: ui Cc:

Description

With 4.5 the widgets no longer work in the sidebar, and are pushed down along the right side to under the post content.

I've attached screenshots of how Twenty Eleven (2.4) looks in both WordPress 4.4 and 4.5

Attachments (7)

4.4.PNG (57.1 KB) - added by Clorith 8 years ago.
The correct display in WordPress 4.4
4.5.PNG (45.9 KB) - added by Clorith 8 years ago.
The new look in WordPress 4.5 (had to zoom out a bit to get the widget included while retaining the post content as in 4.4)
36510.patch (724 bytes) - added by davidakennedy 8 years ago.
Remove singular class if it exist, then add it where needed in twentyeleven_body_classes.
36510.2.patch (1.1 KB) - added by davidakennedy 8 years ago.
Remove singular class if it exist, only in WordPress 4.4 and above.
36510.diff (1.3 KB) - added by swissspidy 8 years ago.
36510.2.diff (1.1 KB) - added by flixos90 8 years ago.
remove singular class, but not the get_body_class() unit test
36510.3.diff (1.2 KB) - added by swissspidy 8 years ago.

Download all attachments as: .zip

Change History (67)

@Clorith
8 years ago

The correct display in WordPress 4.4

@Clorith
8 years ago

The new look in WordPress 4.5 (had to zoom out a bit to get the widget included while retaining the post content as in 4.4)

#1 @eceleste
8 years ago

This is more than a change in the position of the sidebar down from beside to below the content. The edit button has also moved up from below to beside the content. In other words, the content of this template now looks and is positioned in the same way as it would be in the "default" template. It appears somehow the content on a page with sidebar is getting the same formatting that a page without sidebar would get. This happens whether using the prior (v.2.3) version of the theme or the new (v.2.4) version of the theme with 4.5.

#2 @eceleste
8 years ago

This appears to be due to the assignment of a singular class to the body of certain pages in 4.5 (see 35164). Note that TwentyEleven assumes it can prevent the singular class from being assigned in its body_class action. See lines 700-701 of its functions.php file:

	if ( is_singular() && ! is_home() && ! is_page_template( 'showcase.php' ) && ! is_page_template( 'sidebar-page.php' ) )
		$classes[] = 'singular';

Since 4.5 adds the singular class itself, twentyeleven fails to not to add it. To fix this, twenty eleven would have to remove the singluar class. The theme will have to reverse its logic and remove this singular class in these cases.

	if ( is_home() || is_page_template( 'showcase.php' ) || is_page_template( 'sidebar-page.php' ) ) {
		$index = array_search('singular', $classes);
		if ( $index !== false ) unset($classes[$index]);
	}
Last edited 8 years ago by eceleste (previous) (diff)

#3 @SergeyBiryukov
8 years ago

Got a similar report of the sidebar being pushed down due to .singular class in Admired theme.

#4 @SergeyBiryukov
8 years ago

Introduced in [36112].

This ticket was mentioned in Slack in #core by ipstenu. View the logs.


8 years ago

#6 @SergeyBiryukov
8 years ago

  • Milestone changed from Awaiting Review to 4.5.1

Let's investigate possible options.

#7 @hmabpera
8 years ago

I just downloaded WordPress 4.5 and see the same problem where the sidebar layout is not lined up correctly. The main body frame of the page is too low and too far left (overlapping the sidebar menus)
See www.bpera.org

Is there a correction for this behavior? Thanks. HMA

#8 @SergeyBiryukov
8 years ago

A workaround:

function wp36510_remove_singular_class( $classes ) {
	$index = array_search( 'singular', $classes );
	if ( false !== $index ) {
		unset( $classes[ $index ] );
	}

	return $classes;
}
add_filter( 'body_class', 'wp36510_remove_singular_class' );

#9 @eceleste
8 years ago

Yes, this is the workaround I am currently using myself. However, there may be other themes affected is subtle ways. I wonder if we should consider renaming the class introduced in 4.5? Perhaps if it were slightly different (like issingular or page-type-singular instead of simply singular) it might clash with fewer existing themes.

#10 follow-up: @hmabpera
8 years ago

I really need the workaround. Can I just add this function to my functions.php file in the 2011 Child theme or is there more to it than this? thanks

#11 in reply to: ↑ 10 @SergeyBiryukov
8 years ago

Replying to hmabpera:

Can I just add this function to my functions.php file in the 2011 Child theme or is there more to it than this?

Yes, that should work.

#12 @hmabpera
8 years ago

eceleste and SergeyBiryukov, You are BOTH wonderful. I added your function to my functions.php file and it works. This will get me thru tomorrow at least. Thank you sooooo much. bpera

#13 @davel567
8 years ago

Thanks, worked for me too; much appreciated.

#14 @esp2013
8 years ago

Oh thank you! I'm running Twenty Eleven Child Theme on a very complex educational site and was beginning to wonder what I was going to do as my styling was completely messed up after 4.5. This function went a long way to a complete fix but there are still some image size issues - they are appearing smaller than they were before.

#15 @jonnexen
8 years ago

Helped for me, too! :-)

Will this issue be fixed soon? What do you think?

Best Regards

@davidakennedy
8 years ago

Remove singular class if it exist, then add it where needed in twentyeleven_body_classes.

@davidakennedy
8 years ago

Remove singular class if it exist, only in WordPress 4.4 and above.

#16 follow-ups: @davidakennedy
8 years ago

Thanks everyone for reporting this, and digging into it. Thanks @eceleste for the nice investigative work!

I've added two patches with different approaches. There may be better ways to tackle this, but I wanted to throw a few possibilities out there. The challenge here is that we don't want to assume someone has 4.4 or above installed. This should work all the way back to the version of WordPress that Twenty Eleven launched with.

The first patch is probably better, since it has everything in one place. The second one only runs on WordPress 4.4 and above, when the singular class change was merged in. I've tested both on WordPress 4.5 and WordPress 4.3.

Feedback welcome! I can also add another patch here to update the version number, changelog, etc. when we decide on an approach. I'd like to see this fixed as soon as possible.

#17 @davidakennedy
8 years ago

  • Keywords has-patch 2nd-opinion needs-testing added

#18 follow-up: @obenland
8 years ago

Since this is not an issue that is limited to Twenty Eleven it probably needs fixing in core.

#19 @SheldonNesdale
8 years ago

Yay! Thanks team. I just rolled out your fix onto 30 websites. Now they are back to normal. Whew!

#20 @Blog9477
8 years ago

I want to thank everyone for your efforts and @eceleste for finding the problem/fix for the 2011 theme which I used to successfully fix my site. I hope once a fix is decided on, (TwentyEleven 2.4.1?) users will be advised on whether or not they should go back into the theme and undo the fix @eceleste came up with before doing any future update. I for one, am not using a child theme.

#21 in reply to: ↑ 16 @scottkr24
8 years ago

I took my functions.php file and commented out this

function twentyeleven_body_classes( $classes ) {

	if ( function_exists( 'is_multi_author' ) && ! is_multi_author() )
		$classes[] = 'single-author';

	if ( is_singular() && ! is_home() && ! is_page_template( 'showcase.php' ) && ! is_page_template( 'sidebar-page.php' ) )
		$classes[] = 'singular';

	return $classes;
}
add_filter( 'body_class', 'twentyeleven_body_classes' );

Then dropped in what you posted. Went to look at the site, it was blank, nothing was there.

Replying to davidakennedy:

Thanks everyone for reporting this, and digging into it. Thanks @eceleste for the nice investigative work!

I've added two patches with different approaches. There may be better ways to tackle this, but I wanted to throw a few possibilities out there. The challenge here is that we don't want to assume someone has 4.4 or above installed. This should work all the way back to the version of WordPress that Twenty Eleven launched with.

The first patch is probably better, since it has everything in one place. The second one only runs on WordPress 4.4 and above, when the singular class change was merged in. I've tested both on WordPress 4.5 and WordPress 4.3.

Feedback welcome! I can also add another patch here to update the version number, changelog, etc. when we decide on an approach. I'd like to see this fixed as soon as possible.

Last edited 8 years ago by SergeyBiryukov (previous) (diff)

#22 @scamartist26
8 years ago

I want to note that I am directing folks from the stackexchange site to this ticket directly, and they are hopeful to see its resolution in core: http://wordpress.stackexchange.com/questions/223732/updating-to-version-4-5-bumped-my-main-sidebar-widget-out-of-place/223744#223744

#23 in reply to: ↑ 18 @scamartist26
8 years ago

Replying to obenland:

Since this is not an issue that is limited to Twenty Eleven it probably needs fixing in core.

+1 it is a core bug

#24 follow-up: @obenland
8 years ago

This is a pretty bad, if not terrible, experience for Twenty Eleven users, a fairly large pool of WordPress users.
I'd like to ship a new minor version of Twenty Eleven today to fix the problem at least for this theme and all child themes. Once we fixed it in core, we would then ship another minor version with WordPress 4.5.1, removing the hot fix.

#25 in reply to: ↑ 24 @davidakennedy
8 years ago

Replying to obenland:

This is a pretty bad, if not terrible, experience for Twenty Eleven users, a fairly large pool of WordPress users.
I'd like to ship a new minor version of Twenty Eleven today to fix the problem at least for this theme and all child themes. Once we fixed it in core, we would then ship another minor version with WordPress 4.5.1, removing the hot fix.

Any preference or feedback on the patches? Happy to help ship something today.

#26 @krazykatz911
8 years ago

I was having this problem too. For some reason I was able to fix it by deactivating twenty eleven theme extensions, installing simple page sidebars. Activating. Going to pages edit, installing sidebar on all pages. THEN.... deactivating and deleting the simple page sidebars, then reactivating my twenty eleven theme extensions and it magically fixed itself!!!

I have no idea how or why though.

Last edited 8 years ago by krazykatz911 (previous) (diff)

#27 @andreas1974
8 years ago

I'd be happy for any patch to install that makes it look alright again.

Thanks, Andreas

#28 @krazykatz911
8 years ago

Try what I did...

#29 @scottkr24
8 years ago

How do you add the patch?

I'd like to add a couple screen shot images but I don't see a way to do that.

#30 @krazykatz911
8 years ago

Hmmm My fix worked, but if you add any new pages, it overlaps again on the new page. Old pages are ok though.

Last edited 8 years ago by krazykatz911 (previous) (diff)

#31 follow-ups: @krazykatz911
8 years ago

Place in functions:

/**
Fix for 4.5 update overlapping
*/
function wp36510_remove_singular_class( $classes ) {
	$index = array_search( 'singular', $classes );
	if ( false !== $index ) {
		unset( $classes[ $index ] );
	}

	return $classes;
}
add_filter( 'body_class', 'wp36510_remove_singular_class' );
/** End of Fix for 4.5 Update overlapping */

#32 in reply to: ↑ 16 @eceleste
8 years ago

@davidakennedy, I prefer the first patch. It seems much cleaner to just do away with the WP4.5 version of the singular class and then allow TwentyEleven to do its own thing. This is also a safer model for any other theme authors out there who might get caught in the same boat.

The second patch idea looks reasonable but complex, it would be more mysterious to the untrained eye and a less effective model for others.

Replying to davidakennedy:

I've added two patches with different approaches.

#33 in reply to: ↑ 31 @eceleste
8 years ago

@krazykatz911, I think your patch may not always work because you do not specify a priority for your add_filter. You want to be sure this function gets called before TwentyEleven tries to add the singular class, so you probably need to give it a lower priority. Since the add_filter in the theme is at the default priority of 10, you should give yours a lower priority, like this...

add_filter( 'body_class', 'wp36510_remove_singular_class', 1 );

#34 @andbar
8 years ago

Could someone eventually be so kind to explain to a novice how to best apply:

36510.patch

?

As I am also affected by the problem on http://www.bartelsfoto.com

And it's the first time ever I've had any troubles when upgrading my WP, so I am a little lost with this...

TIA, Andreas.

#35 @rebop
8 years ago

Thanks for everyone;s work here. I spent MANY hours looking for missing closing <div tags. Finally restored from a previous backup. Looking forward to the fix.

~Bob

@swissspidy
8 years ago

#36 follow-ups: @swissspidy
8 years ago

Despite the singular body class being useful, I'm currently in favour of fixing this issue by reverting [36112]. Just too many themes add it on their own, as we can see here in the case of Twenty Eleven.

The class hasn't been announced publicly AFAIK, so it wouldn't be a big deal. If one really wants to target all single post objects (which .singular does, after all), you can just target .single, .page, .attachment.

So just 1 revert that would go into 4.6 and 4.5.1 instead of releasing new versions of the themes or something like that.

See 36510.diff.

#37 in reply to: ↑ 31 @scottkr24
8 years ago

This worked perfectly for me, thanks.

Replying to krazykatz911:

Place in functions:

/**
Fix for 4.5 update overlapping
*/
function wp36510_remove_singular_class( $classes ) {
	$index = array_search( 'singular', $classes );
	if ( false !== $index ) {
		unset( $classes[ $index ] );
	}

	return $classes;
}
add_filter( 'body_class', 'wp36510_remove_singular_class' );
/** End of Fix for 4.5 Update overlapping */

#38 @Blog9477
8 years ago

OK, so it looks like its 50/50 for changes to WP core versus release TwentyEleven 2.4.1 etc - (changing WP core makes more sense to me because then it is a non issue for any theme, forevermore) - but either way how do you get someone at WP to pull the trigger and just do it? The WP world needs this fixed, and IMHO non coders should not have to go in an install manual hot patches to keep their sites working!

#39 follow-up: @eceleste
8 years ago

@swisspidy, this does feel like something best fixed in core, though I can live with the theme patching if that's what maintainers decide to opt for. I would just keep in mind that TwentyEleven has been a model for a long while and there are probably many themes derived from it that will need patching, beyond just the one WP distributes.

One option short of a full reversion, though, would be to simply assign a different name to the class in WP core. WP already uses some might awkward class names, this could be another of those. For example, post-type-singular. Then again, I really have no use for this myself, so I won't fight hard either way.

How soon can a new version of WP or the theme get distributed? I think a lot of people are feeling the impact of this problem right now.

#40 in reply to: ↑ 36 @Clorith
8 years ago

Replying to swissspidy:

Despite the singular body class being useful, I'm currently in favour of fixing this issue by reverting [36112]. Just too many themes add it on their own, as we can see here in the case of Twenty Eleven.

The class hasn't been announced publicly AFAIK, so it wouldn't be a big deal. If one really wants to target all single post objects (which .singular does, after all), you can just target .single, .page, .attachment.

So just 1 revert that would go into 4.6 and 4.5.1 instead of releasing new versions of the themes or something like that.

See 36510.diff.

Had a quick check, couldn't find any announcement of the new class so I think this would be a safe presumption and most likely (from observation) the best approach.

We don't know if any other themes might have been using their own variation of singular in their styles as well pre 4.5, as it's much more obvious with default themes.

Any new theme that are basing their code off the 4.5 class can very easily implement it them selves using the body_class filter as well so they don't lose out on anything if we revert.

function themeslug_singular_body_tag( $tags ) {
	if ( is_singular() ) {
		$tags[] = 'singular';
	}
	return $tags;
}
add_filter( 'body_class', 'themeslug_singular_body_tag' );

For those asking how to apply patches, it's covered in the handbook at https://make.wordpress.org/core/handbook/tutorials/working-with-patches/

Other than that I'd request that comments not directly relating to the ticket at hand be directed to the forums at https://wordpress.org/support as this ticket is getting a bit messy and hard to keep track of.

#41 in reply to: ↑ 36 ; follow-up: @SergeyBiryukov
8 years ago

Replying to swissspidy:

Despite the singular body class being useful, I'm currently in favour of fixing this issue by reverting [36112]. Just too many themes add it on their own, as we can see here in the case of Twenty Eleven.

I tend to agree.

#42 in reply to: ↑ 39 @swissspidy
8 years ago

Replying to eceleste:

One option short of a full reversion, though, would be to simply assign a different name to the class in WP core. WP already uses some might awkward class names, this could be another of those. For example, post-type-singular. Then again, I really have no use for this myself, so I won't fight hard either way.

Renaming the class in a minor release isn't a good idea, it makes things more complicated for developers. When reverting, we could think about re-adding the class with a different name in 4.6 or so.

How soon can a new version of WP or the theme get distributed? I think a lot of people are feeling the impact of this problem right now.

We're working hard on fixing the bugs that have been reported for 4.5 and will probably share a plan for the 4.5.1 release on Wednesday in the dev chat.

#43 in reply to: ↑ 41 @obenland
8 years ago

Replying to SergeyBiryukov:

Replying to swissspidy:

Despite the singular body class being useful, I'm currently in favour of fixing this issue by reverting [36112]. Just too many themes add it on their own, as we can see here in the case of Twenty Eleven.

I tend to agree.

I too think we should go that route.

#44 @karmatosed
8 years ago

+1 to revert.

#45 in reply to: ↑ 31 @paloprisk
8 years ago

Replying to krazykatz911:

Place in functions:

/**
Fix for 4.5 update overlapping
*/
function wp36510_remove_singular_class( $classes ) {
	$index = array_search( 'singular', $classes );
	if ( false !== $index ) {
		unset( $classes[ $index ] );
	}

	return $classes;
}
add_filter( 'body_class', 'wp36510_remove_singular_class' );
/** End of Fix for 4.5 Update overlapping */

krazykatz911 patch works great for me ... thanks !
The problem I had is shown in the following pic.
Since I upgraded to WP4.5, there was an overlapping of the bbPress pages content *INTO* the left sidebar widget.
This occured on bbPress pages *ONLY* and now, thanks to the patch, it's back to normal !
(hope *NEW* bbPress pages will still be styled the correct way ... we'll see)

https://dl.dropboxusercontent.com/u/29826503/groupez.net/overlap2.jpg

Last edited 8 years ago by paloprisk (previous) (diff)

#46 in reply to: ↑ 31 @alye
8 years ago

Replying to krazykatz911:

Place in functions:

/**
Fix for 4.5 update overlapping
*/
function wp36510_remove_singular_class( $classes ) {
	$index = array_search( 'singular', $classes );
	if ( false !== $index ) {
		unset( $classes[ $index ] );
	}

	return $classes;
}
add_filter( 'body_class', 'wp36510_remove_singular_class' );
/** End of Fix for 4.5 Update overlapping */

Thanks a lot the patch fixed the problem!!!

#47 @swissspidy
8 years ago

  • Keywords commit added; 2nd-opinion removed
  • Owner set to swissspidy
  • Status changed from new to assigned

The consensus is to revert the change.

I'll let this sink in for another day or so and commit 36510.diff afterwards if there are no (unexpected) objections.

#48 @davidakennedy
8 years ago

I wanted to come back and say that after thinking about this over the weekend, I'm also in favor of reverting the change.

This causes quite a big breakage in existing themes, and there's no easy way for singular as a class name to live in Core without possibly creating issues. We could educate existing themers to help them change current themes to accommodate the class, but this change doesn't really provide enough benefit to justify that effort. Plus, the sooner we take it out, the better because fewer new themes will use it.

#49 @eceleste
8 years ago

+1 for reverting the change

This ticket was mentioned in Slack in #bbpress by casiepa. View the logs.


8 years ago

@flixos90
8 years ago

remove singular class, but not the get_body_class() unit test

#51 @flixos90
8 years ago

I think we can leave the general unit test for the singular classes in get_body_class() in Core - actually removing the singular class should be sufficient (see 36510.2.diff) - what do you think @swissspidy?

@swissspidy
8 years ago

#52 @swissspidy
8 years ago

  • Keywords needs-testing removed

@flixos90 Yeah, you're right, that was a rash patch.

Note that you can't use @tickets in the docblock as @ticket is used by phpunit (allows you to do phpunit --group <ticketnumber>. See 36510.3.diff.

This ticket was mentioned in Slack in #core by adamsilverstein. View the logs.


8 years ago

#54 @ocean90
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 37249:

Themes: Revert [36112]

Adding the singular class per default to the list of body classes is breaking the layout of Twenty Eleven and other themes. Twenty Eleven adds the .singular class only to single pages if the page doesn't use specific page templates.

Props flixos90, swissspidy.
Fixes #36510.

#55 @ocean90
8 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

#56 @ocean90
8 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 37250:

Themes: Revert [36112]

Adding the singular class per default to the list of body classes is breaking the layout of Twenty Eleven and other themes. Twenty Eleven adds the singular class only to single pages if the page doesn't use specific page templates.

Merge of [37249] to the 4.5 branch.

Props flixos90, swissspidy.
Fixes #36510.

This ticket was mentioned in Slack in #core by adamsilverstein. View the logs.


8 years ago

#58 follow-up: @andreas1974
8 years ago

I notice now that the issued is set to fixed, and is also closed.
Could someone please explain to me whether the fix will show up as an "update" on the administration pages of my Wordpress installation.

#59 in reply to: ↑ 58 @Clorith
8 years ago

Replying to andreas1974:

I notice now that the issued is set to fixed, and is also closed.
Could someone please explain to me whether the fix will show up as an "update" on the administration pages of my Wordpress installation.

Fixed means the code has been added to the current working copy of WordPress (the development version right now), and will be included in a future update.

In this case that future update will be 4.5.1 as seen from https://make.wordpress.org/core/2016/04/22/4-5-1-release-candidate/

This ticket was mentioned in Slack in #core by ocean90. View the logs.


8 years ago

Note: See TracTickets for help on using tickets.