Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#22230 closed defect (bug) (fixed)

WP adds <br /> before <select even where there is no line break in source code

Reported by: chirael's profile chirael Owned by: nacin's profile nacin
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.4.2
Component: Formatting Keywords: wpautop has-patch commit
Focuses: Cc:

Description

My source code in the WP HTML editor:

<label for="sellby">How soon do you need to sell?</label><select id="sellby" name="sellby" required="1">

Note how there is *no line break* between </label> and <select

HTML generated by WP, however:

<label for="sellby">How soon do you need to sell?</label><br />
<select id="sellby" name="sellby" required="1">

Um, how (and why?) did that extra <br /> get in there?

This also happened when I was using the Visual editor; switched to the HTML editor to see if it was an error on my part but nope - there is no line break whatsoever in my HTML source code, so I have no idea why WP insists on adding one.

It's particularly vexing because I would like my text label to be next to (on the same line as) my select boxes, but WP is forcing it to be on the next line, which is making my form look awful.

AFAIK it's only happening with select boxes, but it's pretty consistent for me.

Since it's happening with the HTML editor as well as the Visual editor I left the Component as General so better minds than mine can categorize it appropriately.

Attachments (5)

22230.patch (951 bytes) - added by rachelbaker 10 years ago.
Removed select tag from $allblocks string in wpautop function.
22230-test.patch (743 bytes) - added by rachelbaker 10 years ago.
Added unit test test_skip_select_elements() to autop tests.
22230.2.patch (1.1 KB) - added by rachelbaker 10 years ago.
Corrected paths in diff file
22230-test.2.patch (929 bytes) - added by rachelbaker 10 years ago.
Corrected paths in diff file to be root of develop repo.
22230.3.patch (1.9 KB) - added by kirasong 10 years ago.
Remove <option> from blocks as well; adjust unit test name to reflect the change.

Download all attachments as: .zip

Change History (18)

#1 @chirael
11 years ago

Also I went into my Wordpress MySQL database and selected the post (page) in question from wp_posts, and in post_content there is no <br /> between the </label> and <select.

So it would appear that the <br /> is somehow added when the page is requested/viewed/rendered by WP for output to the browser.

#2 @toscho
11 years ago

  • Cc info@… added
  • Component changed from General to Formatting

This happens because wpautop() treats select as a block element.

// Space things out a little
$allblocks = '(?:table|thead|tfoot|caption|col|colgroup|tbody|tr|td|th|div|dl|dd|dt|ul|ol|li|pre|select|option|form|map|area|blockquote|address|math|style|p|h[1-6]|hr|fieldset|legend|section|article|aside|hgroup|header|footer|nav|figure|figcaption|details|menu|summary)';

Hm, option too. No idea why.

#3 @chirael
11 years ago

Ok, I guess that's a reason, but I still consider it a bug. Why does WP make that decision for me? I'd really like to have my label be next to (on the same line as) my select box. If I edit something in HTML mode I don't see a good reason for WP to add in code that was not there in the first place.

#4 @chirael
11 years ago

While I still consider this a bug, a workaround which seems to work for me is to use styles to select the br element and then set its display to none:

<li class="nobr"><label for="sellby">How soon do you need to sell?</label><select id="sellby"

and then in the theme's style.css:

li.nobr br { display: none; }

Just wanted to post this hack in case anyone else is vexed by this WP behavior; credit to this page for the idea.

#5 @uecasm
11 years ago

+1 to annoying. It's also adding <p></p> tags around any text immediately following the <select> element, which can't be worked around via the display:none trick (as that hides the text as well).

I strongly suggest reducing wpautop to only treat elements that actually are blocks as blocks.

I further suggest that regardless of whether that is done or not, WP should respect some kind of marker in the post source itself to disable wpautop within a specific section of content without disabling it globally. Perhaps by hijacking the now-obsolete <nobr> tag -- make wpautop strip out the <nobr></nobr> tags themselves but leave any content within those tags completely untouched. This way authors can explicitly indicate sections of a post which contain preformatted HTML that they don't want messed with.

#6 @nacin
10 years ago

  • Keywords wpautop added

#7 @nacin
10 years ago

  • Milestone changed from Awaiting Review to 3.9

We've previously removed input from the list of blocks. I'm fine with also removing select.

@rachelbaker
10 years ago

Removed select tag from $allblocks string in wpautop function.

@rachelbaker
10 years ago

Added unit test test_skip_select_elements() to autop tests.

#8 @rachelbaker
10 years ago

  • Keywords has-patch needs-testing added

Removed select tag from list of block elements in wpautop() and included related unit test.

@rachelbaker
10 years ago

Corrected paths in diff file

@rachelbaker
10 years ago

Corrected paths in diff file to be root of develop repo.

#9 @seanchayes
10 years ago

I tested the patch and found that, with the patch applied, when the <select> is up against the <label> (as in the original request) no <br> is injected between the two.

@kirasong
10 years ago

Remove <option> from blocks as well; adjust unit test name to reflect the change.

#10 @kirasong
10 years ago

  • Keywords commit added; needs-testing removed

I ran the unit test and realized it did not pass with the patch applied.

Noticed this is the case because <option> is also in the list of block elements.

In 22230.3.patch:

  • Refreshed.
  • Removed <option> from the list of block elements.
  • Renamed unit test to reflect the testing of both elements.

We could also create separate tests for <select> and <option>, but because they'll most often be found together, I think this is fine.

#11 @cinemafunk
10 years ago

I had the same issues with the select and label. I added a <div class="nobr"> that surrounded it, targeted that with div.nobr br {display:none;} in the css and everything looks gorgeous. Thanks!

#12 @nacin
10 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 27761:

Remove select and input from wpautop()'s HTML blocks list.

props rachelbaker, DH-Shredder.
fixes #22230.

#13 @wonderboymusic
10 years ago

In 28853:

Fix wpautop() unit tests. See #25646, #22230, #25856.

Props rachelbaker.
Fixes #28638.

Note: See TracTickets for help on using tickets.