wp_nav_menu outputs tags without line breaks in 4.4, causes strange bug with justified text

[wp-architect]

After updating from 4.3.1 to 4.4, justified alignment being applied via CSS to my main navigation no longer worked.

After inspecting the problem, It turns out 4.4 was displaying HTML tags on the same line which causes a strange bug in Google Chrome on latest OSX (untested in other browsers). See stackoverflow: ​https://stackoverflow.com/questions/24781842/justified-horizontal-list-with-minified-html

Reapplying line breaks to the menu output solved this issue.

Proposed fix - NOTE: I am a first time contributor and would like the opportunity to submit a pull request for this small fix, if it is legitimate.

Restore the following Walker_Nav_Menu function in wp-includes/nav-menu-template.php from 4.3 -

<?php
/**
         * Ends the element output, if needed.
         *
         * @see Walker::end_el()
         *
         * @since 3.0.0
         *
         * @param string $output Passed by reference. Used to append additional content.
         * @param object $item   Page data object. Not used.
         * @param int    $depth  Depth of page. Not Used.
         * @param array  $args   An array of arguments. @see wp_nav_menu()
         */
        function end_el( &$output, $item, $depth = 0, $args = array() ) {
                $output .= "</li>\n"; // added \n to appear on new lines
        }

Are you using either the latest version of WordPress, or the latest development version? If not, please update first.

Yes, this occurs on 4.4

What steps should be taken to consistently reproduce the problem?

Activate any theme that uses the wp_nav_menu. View site and see source code for navigation output.

Does the problem occur even when you deactivate all plugins and use the default theme?

Yes

In case it's relevant to the ticket, what is the expected output or result? What did you see instead?

4.3

<div class="menu">
<ul>
<li><a href="#">Menu Item</a></li>
<li><a href="#">Menu Item</a></li>
<li><a href="#">Menu Item</a></li>
<li><a href="#">Menu Item</a></li>
</ul>
</div>

4.4

<div class="menu"><ul><li><a href="#">Menu Item</a></li><li><a href="#">Menu Item</a></li><li><a href="#">Menu Item</a></li><li><a href="#">Menu Item</a></li></ul></div>

[SergeyBiryukov]

Hi @wp-architect, welcome to Trac!

Thanks for the report. Introduced in [34321], see #27762.

[DoodleDogCody]

Would like to add to this with code example of why this is a problem. On a number of mys sites I do the following

.nav-primary ul.menu-primary {
    text-align: justify;
}
.nav-primary ul.menu-primary::after {
    content: " ";
    display: inline-block;
    width: 100%;
}

This has the effect of evenly spacing the nav menu items within their container. This only works when there is space between the <li> tags because justify only works if there are multiple words or in this case elements for it to justify.

[SergeyBiryukov]

Could we perhaps add a space instead of bringing back \n?

[DoodleDogCody]

@SergeyBiryukov Yes I just ran a quick test in the inspector and a space works as well. I assume this is the same kind of thing that @wp-architect is having problems with and a space should fix his problems as well.

I assume the change was simply to help lower page size a bit by having elements on the same line so adding a space should accomplish that goal without hurting functionality.

[ocean90]

A space would produce the same issues as a newline character, see ​https://css-tricks.com/fighting-the-space-between-inline-block-elements/ for background info.

[SergeyBiryukov]

Replying to DoodleDogCody:

I assume the change was simply to help lower page size a bit by having elements on the same line so adding a space should accomplish that goal without hurting functionality.

Per #27762, it was to avoid spacing between menu items when displaying them horizontally.

[DoodleDogCody]

After reading #27762 I see this was done for a specific reason of not relying on Floats for horizontal nav menus.

That ticket mentions edge cases for display inline-block. I assume there are other ways to accomplishing the goal of justifying nav menus using things like flexbox but I feel this change negatively effects sites that have relied on such css like listed above in my first comment.

[obenland]

Replying to DoodleDogCody:

That ticket mentions edge cases for display inline-block.

Using inline-block is far from being an edge case.

In all my time dealing with themes, reviewing themes, and making them, I have not once seen a justified menu. Even visiting "regular" websites, I don't think I ever noticed one. This just to explain why that was not caught by me in the initial work on the ticket—it just did not think of displaying it that way.

I guess we'll have to deal with floats a while longer.

[wp-architect]

Replying to DoodleDogCody:

Would like to add to this with code example of why this is a problem. On a number of mys sites I do the following

.nav-primary ul.menu-primary {
    text-align: justify;
}
.nav-primary ul.menu-primary::after {
    content: " ";
    display: inline-block;
    width: 100%;
}

This has the effect of evenly spacing the nav menu items within their container. This only works when there is space between the <li> tags because justify only works if there are multiple words or in this case elements for it to justify.

Thanks for mentioning that @DoodleDogCody - this is actually the exact method I am using. It works fairly well as an approach to responsive layout since Flexbox is still not widely adopted by browsers. Switching over to floats in an option but requires careful calculations when it comes to padding/margins between elements whereas text-align:justify figures it out for you.

For some added context, here is an article which further explains this approach. ​http://www.barrelny.com/blog/text-align-justify-and-rwd/

[peterwilsoncc]

35107.diff​ is an approach to allow both approaches:

• remove \n, \r, \t string replacement in wp_page_menu since [9246]
• Introduces item_before and item_after arguments to
  • wp_nav_menu
  • wp_list_pages
  • wp_page_menu
• item_before defaults to an empty string
• item_after defaults to a line-break to match the pre 4.4 behaviour

The first item has a reasonably heavy touch, possibly too much for a point release.

[DoodleDogCody]

Replying to peterwilsoncc:

35107.diff​ is an approach to allow both approaches:

• remove \n, \r, \t string replacement in wp_page_menu since [9246]
• Introduces item_before and item_after arguments to
  • wp_nav_menu
  • wp_list_pages
  • wp_page_menu
• item_before defaults to an empty string
• item_after defaults to a line-break to match the pre 4.4 behaviour

The first item has a reasonably heavy touch, possibly too much for a point release.

I would say +1 to this idea. I think it's the best way to accomplish this. Allows default to go back to what it was before the update and gives people the options of easily editing with a simple filter in functions.php file.

[obenland]

I think for 4.4.1 we should just revert the change. It would have been nice to catch that before release. It was introduced in the first third of the release cycle.

[peterwilsoncc]

Replying to obenland:

I think for 4.4.1 we should just revert the change.

Yep, that works. I've forked the enhancement that is my patch to #35206.

[obenland]

In 36081:

Menus: Bring back line break between menu items.

While removing line breaks was great for all use cases except justified menu
items, it broke menus with justified menu items.
Reverts [34321].

Props wp-architect.
Fixes #35107.

[obenland]

In 36082:

Menus: Bring back line break between menu items.

While removing line breaks was great for all use cases except justified menu
items, it broke menus with justified menu items.
Reverts [34321].

Merges [36081] to the 4.4 branch.
Props wp-architect.
Fixes #35107.