| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | filter.module |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Here are the issues:
1. Using Filtered HTML input format comments are removed. I think it shouldn't do this or it should allow the !-- tag to be added (it doesn't do that either).
2. If the comments have some html tags inside, the result is even worse. <!-- comment <p>comment</p> --> will result in comment -->. If my previous statement is arguable, now for sure something is wrong. It should either remove the comment or (ideally IMO) let it untouched.
3. Finally, using Full HTML will not strip the comment, but because of the line brake filter if you write
<!-- comment -->
<!-- comment <p>comment</p> --><p><!-- comment --><br /><!-- comment
<p>comment</p>
<p> --></p>The patch gives you the option to add <!--> as an allowed tags. If the tag is not allowed the comment is removed for good, with everything inside it, html code included (takes care of problem #1 and #2)
It ignores html comments when doing the autop processing (problem #3).
I removed the test because... it will not pass. DOMDocument treats comments as they are, which is text so it will not add a rel="nofollow" to a link inside a comment. That test now passes only because the comments tags are removed and the link is actually displayed (problem #2).
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| html-filter-and-comments.patch | 4.24 KB | Idle | Failed: Failed to apply patch. | View details |
Comments
#1
The last submitted patch failed testing.
#2
#3
Important backgrounds (duplicates?):
#69430-3: Comments containing HTML are broken (only leading tag is stripped)
#103563: HTML filter escaping html comments
#4
The last submitted patch failed testing.
#5
HEAD is broken.
#6
+++ includes/common.inc 28 Aug 2009 08:50:30 -0000@@ -1503,11 +1505,20 @@
+ $comment = &$matches[4];
+
+ if ($comment) {
+ $elem = '!--';
+ }
...
+
+ if ($comment) {
+ return $comment;
+ }
Needs inline comments explaining what's being done here.
+++ modules/filter/filter.module 28 Aug 2009 08:50:30 -0000@@ -954,7 +954,8 @@
// Opening or closing tag?
- $open = ($chunk[1] != '/');
+ $open = ($chunk[1] != '/' || $chunk[1] != '!');
+ $comment = (substr($chunk, 0, 4) == '<!--');
Likewise, inline comment needs to be updated.
+++ modules/filter/filter.module 28 Aug 2009 08:50:30 -0000@@ -963,7 +964,7 @@
// Only allow a matching tag to close it.
- elseif (!$open && $ignoretag == $tag) {
+ elseif ((!$open && $ignoretag == $tag) || $comment) {
ditto, explaining special case of $comment
I'm on crack. Are you, too?
#7
#222926: HTML Corrector filter escapes HTML comments
#8
Re-opening per #222926-119: HTML Corrector filter escapes HTML comments. The point being that with the refactoring of the HTML corrector in #374441: Refactor Drupal HTML corrector (PHP5) it is a different beast in 7.x. Specifically,
1) in 6.x HTML comments get escaped, whereas in 7.x they get removed (i.e. #374441 partially fixed the problem)
2) this issue and #222926 need different approaches to fix them
#9
subscribe
#10
subscribe
#11
Better title. Major, as this is regression from D6.
#12
1) Apparently, there almost no tests for the Line break filter. Bad!
2) For filter_xss(), we have lots of XSS tests, but not a single assertion that verifies that non-XSS stuff stays as is. Bad!
3) FilterUnitTestCase contains old and duplicated tests for check_plain(), which is already tested elsewhere. If required, I'm going to move those clean-ups into a separate issue.
#13
This looks good. Committed to CVS HEAD.
#14
Automatically closed -- issue fixed for 2 weeks with no activity.