Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Is there a reason not to allow the HTML font tag in filter_xss_admin()? When letting admin's use TinyMCE for the contact form header text, they cannot color font using font tags in the current implementation.
| Comment | File | Size | Author |
|---|---|---|---|
| filter.module_25.patch | 1.31 KB | RobRoy | |
Comments
Comment #1
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedthe reason is "don't use the font tag"
http://www.webreference.com/html/tutorial22/
Comment #2
RobRoy CreditAttribution: RobRoy commentedOf course, I completely agree with you that the font tag should never be intentionally used and the thought of it makes me shudder and think back to 1992.
BUT I do still think that we should allow the font tag for filter_xss_admin() since its purpose is to prevent XSS attacks, not enforce "progressive" HTML. For situations when a client wants to use a WYSIWYG editor for the contact settings header, any font tags will be stripped. Just including the font tag in the allowed list doesn't mean we are advocating the use of it in the content. :P
Comment #3
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedMoving this to HEAD to get more reviews.
Comment #4
Steven CreditAttribution: Steven commentedfilter_xss_admin() also removes inline CSS. Removing the font tag is really no different.
Comment #5
RobRoy CreditAttribution: RobRoy commentedWell, with inline CSS you could do a lot more damage than with the font tag. For some rich-text editors, it would be nice to allow the font tag. With inline CSS you could totally bork a site. I really see no harm in including font. If you show me some sort of "xss" vulnerability with the font tag, then I agree. But since this function is just supposed to filter xss or other harmful attacks, IMHO we should allow font.
Comment #6
drummWhich WYSIWYG editors?
Comment #7
RobRoy CreditAttribution: RobRoy commentedI encountered this with TinyMCE when a client was trying to change the color of some text and it kept reverting on save.
I really think that 'font' should be allowed. Unless someone tells me how 'font' could be used for an XSS attack, I think this should go in. Again, I agree that 'font' is horrible and should never be voluntarily used, but AFAICT it poses no XSS threat and this is just a political argument because 'font' sucks.
Comment #8
drummComment #9
StevenPatzpatch looks good and works as advertised.
Comment #10
Dries CreditAttribution: Dries commentedI've committed this just for the sake of getting some WYSIWYG editors to work. Plus, the font-tag is part of the XHTML standard. Might want to commit this to D5 too.
Comment #11
Dries CreditAttribution: Dries commentedI rolled back this patch. Font is deprecated. Better to report this bug to the maintainers of the wysiwyg package.
Comment #12
chx CreditAttribution: chx commentedNot just that it's deprecated, it's totally omitted from XHTML Strict.
Comment #13
m1mic CreditAttribution: m1mic commentedFrom looking at this patch, it appears that it is attempting change the filter so that the font tag isn't removed from the underlying html. Moxie Code's TinyMCE has the ability to convert the deprecated font tags into inline styles. In the TinyMCE 5.x-dev module, the settings are in place for this to happen. If the admin goes to admin/settings/tinymce/edit/[enter profile name here] and goes to the Cleanup and Output section, there is the option to Convert font tags to styles. The trouble with this is that it doesn't actually work.
With the help of mlsamuelson, we came up with a patch (see http://drupal.org/node/126364) that corrects this bug. Using inline styles (inside of span tags) instead of font tags will make it validate as XHTML Strict.
Comment #14
Mupsi