check_markup(empty string) = [#100679]

This is when used with a format filter that implements 'Line break converter"
_filter_autop('') == "br" tag.
the doc for the function says :
"Convert line breaks into [p] and [br] in an intelligent fashion.
* Based on: http://photomatt.net/scripts/autop"

The above link reads
"Basically it takes PHP’s nl2br function to the logical next step and converts double line breaks to paragraphs where applicable, does line breaks as before, and best of all it’s aware of block-level HTML tags so it won’t mess up your page"
Yet we have nl2br('') == ''.

So I'm not sure 1) if it's a bug or a feature 2) where it should be corrected if necessary.

I noticed this on an extraneous br tag being output when a rendering a node with no body.

Comment	File	Size	Author
#12	filter_14.patch	1.21 KB	RobRoy
#12
#5	filter.module_26.patch	1.19 KB	yched
#5

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

yched CreditAttribution: yched commented 4 December 2006 at 01:40

Title:

check_plain('') = "br" tag with "Line break converter" filter

» check_plain(empty string) =

Comment #2

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 4 December 2006 at 03:04

I've noticed this too, and would consider it a bug. At one point I'd filed an issue about this but I can't seem to find it now, and in any case this is a better description than mine. ;) I believe I'd traced it to the filter_xss stuff, which got into Scary Regex Hell (tm) which was beyond my level of debugging skills.

Comment #3

Steven CreditAttribution: Steven commented 4 December 2006 at 18:56

Title:

check_plain(empty string) =

» check_markup(empty string) =

Correcting title. Check_plain() will never add HTML to the output.

Comment #4

yched CreditAttribution: yched commented 4 December 2006 at 21:32

Er, right, stupid mistake - sorry.

Comment #5

yched CreditAttribution: yched commented 5 December 2006 at 01:59

Status:

Active

» Needs review

File	Size
filter.module_26.patch	1.19 KB

I think I nailed this to the following lines in _filter_autop :

$chunk = preg_replace('/\n?(.+?)(?:\n\s*\n|\z)/s', "<p>$1</p>\n", $chunk); // make paragraphs, including one at the end
$chunk = preg_replace('|<p>\s*?</p>|', '', $chunk); // under certain strange conditions it could create a P of entirely whitespace

1st line creates some ...\n blocks.
2nd line corrects some seemingly "strange" effects in this first line where empty  are created and removes them _but_ forgets the trailing \n in the capturing pattern.

Attached patch corrects this.

Side note : this should also correct the behaviour where a   gets inserted between two <pre> blocks following each other
Demo right below :
<code>aabb
outputs : (also note the   that gets added after the second <pre>)
aabb

Comment #6

anders.fajerson CreditAttribution: anders.fajerson commented 6 December 2006 at 06:58

Status:

Needs review

» Needs work

I still get a br tag, both on a custom module which allows empty bodies and when erasing the body for a page content type in the db.

Comment #7

yched CreditAttribution: yched commented 6 December 2006 at 12:52

Status:

Needs work

» Needs review

fajerstarter : I don't see that myself (with that patch).
Can you provide additional info on your filters configuration ?
What are the filters (and their settings) your content goes through ?

This patch deals with a bug in the "Line break converter" filter, and AFAICS, solves it. Maybe you're experiencing a related but different bug ?

Comment #8

anders.fajerson CreditAttribution: anders.fajerson commented 6 December 2006 at 15:37

I had no idea that the input format was "cached". When re-saving the input formats (standard; Filtered and full HTML) it works as advertised, took a while until I figured that out.. Can that extra step be dealt with in the patch?

Comment #9

yched CreditAttribution: yched commented 6 December 2006 at 21:06

Yes, it's only after replying to you and leaving home that I thought about cached filter results - sorry about that.

So, to anyone willing to test the patch : you'll have to empty your cache after applying.

PS : no, the patch itself can't deal with this - maybe it this gets in an update function could ba added to empty the cache (but I doubt it :-) or else system.install would be overcrowded by simple 'empty cache' updates).