Filter module Line Break convertor adds

tag around an

element. This can be fixed by modifying the filter.module file. The function to be modified is _filter_autop, and the fix is to add iframe to the $block regular expression so that the filter regards iframes as block-level elements, which is their primary use. Original code snippet: 
function _filter_autop($text) {
  // All block level tags
  $block = '(?:table|thead|tfoot|caption|colgroup|tbody|tr|td|th|div|dl|dd|dt|ul|ol|li|pre|select|form|blockquote|address|p|h[1-6]|hr)';
Patched code snippet: 
function _filter_autop($text) {
  // All block level tags
  $block = '(?:iframe|table|thead|tfoot|caption|colgroup|tbody|tr|td|th|div|dl|dd|dt|ul|ol|li|pre|select|form|blockquote|address|p|h[1-6]|hr)';
A sophisticated fix would be to include configuration settings for the filter module to change behavior for the elements used as either inline / block-level elements, like listed in http://htmlhelp.com/reference/html40/block.html -page list on the bottom of the page.
CommentFileSizeAuthor
#5 810824-v2.patch2.37 KBdfletcher
#3 810824-v1.patch2.15 KBdfletcher

Comments

mongeron’s picture

mongeron commented

The first few lines should read:

Filter module Line Break convertor adds "p" tag around an "iframe" element.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: Filter module adds <p></p> around <iframe> elements » Line break filter adds paragraphs around <iframe> elements
Version: 6.16 » 7.x-dev
dfletcher’s picture

dfletcher commented
Status: Active » Needs review
Issue tags: +Needs tests
StatusFileSize
new 810824-v1.patch2.15 KB

According to the unit tests, iframe should not be a block element but rather one of the elements that is ignored entirely, a few lines below the $block = line pointed out by mongeron.

I think the "sophisticated fix" mentioned in OP should be a separate feature request, and this bug should be closed with the simpler solution in the patch.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Needs work 
+++ modules/filter/filter.module	10 Feb 2011 20:29:18 -0000
@@ -1590,11 +1590,11 @@ function _filter_autop($text) {
-  // Split at opening and closing PRE, SCRIPT, STYLE, OBJECT tags and comments.
+  // Split at opening and closing PRE, SCRIPT, STYLE, OBJECT, IFRAME tags and comments.

Exceeds 80 chars.

+++ modules/filter/filter.test	10 Feb 2011 20:29:19 -0000
@@ -833,6 +833,10 @@ class FilterUnitTestCase extends DrupalU
+        "<p>aaa</p>\n<p><iframe>aaa</iframe></p>\n<p>bbb</p>" => FALSE,

Let's remove the outer, irrelevant markup and only test the actual IFRAME tag and its direct siblings.

A second, positive assertion to verify that we're not adding paragraphs to the inner HTML of the IFRAME would be good to add.

Powered by Dreditor.

dfletcher’s picture

dfletcher commented
Status: Needs work » Needs review
StatusFileSize
new 810824-v2.patch2.37 KB

Comments fixed. Some lines below my change were also > 80, so I reformatted the whole paragraph.

Test simplified:

"<iframe>aaa</iframe>\n\n" => array(
  "<p><iframe>aaa</iframe></p>" => FALSE,
),

Sun: There already a positive assertion test, line 819 of filter.test. It accidentally does not get triggered because of http://drupal.org/node/1054632 , which I discovered while working on that bug.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Closed (duplicate)

This is now handled in #1063178: Line break filter will ignore everything following a <pre>xxx</pre>.