Upgraded from 6.x-2.8.

I get this SGML Parser validation error in Firefox. It has something to do with alt="share". It should be quotation marks not ".

Not critical, but really annoying.

line 283 column 121 - Error: an attribute value must be a literal unless it contains only name characters

onclick="return addthis_sendto()"><img src="http://s9.addthis.com/button1-share.gif" width="125" height="16" alt=&quot;share&quot; /></a><script type="text/javascript" src="http://s7.addthis.com/js/152/addthis_widget.js"></script></div>
CommentFileSizeAuthor
#6 addthis_fixandfeatures_20100303a.patch3.96 KBgregarios

Comments

mlaw’s picture

mlaw commented

It changed my formatting. It should've been:

I get this SGML Parser validation error in Firefox. It has something to do with:

alt=&quot;share&quot;

It should be quotation marks not

&quot;
gregarios’s picture

gregarios commented
Status: Needs work » Active

This is a definite bug. Too bad they didn't implement the patch I gave them. Anything added to the "Button image HTML attributes" setting now gets its quotes converted. This should not happen.

mlaw’s picture

mlaw commented

gregarios, i saw your issue here: http://drupal.org/node/686426

any hack or patch that can fix this for now?

mlaw’s picture

mlaw commented
Component: Miscellaneous » Code
gregarios’s picture

gregarios commented

any hack or patch that can fix this for now?

I think the security problem may be that they were allowing the quotes to begin with, which meant that the text people place in the site via that "Button image HTML attributes" setting was unfiltered. They need to break-up the blanket idea of "fill-in-the-textfield with whatever you want in the html code" and make several very specific settings instead that allow plaintext in the fields. They need to replace that setting with 4 different settings like these:

  • Alt: ________
  • Id: ________
  • Class: ________
  • Name: ________

I don't have a patch worked up for it, but if the maintainer is good enough he/she should be able to crank that out pretty easily. It would allow all the tags, while maintaining security against cross-site scripting.

gregarios’s picture

gregarios commented
Status: Active » Patch (to be ported)
StatusFileSize
new addthis_fixandfeatures_20100303a.patch3.96 KB

Ok, so attached is a patch that adds the new settings fields for the Alt tag and Class tags. Apply it to the 6.x-2.9 version. Please, maintainers, implement this if you would. There is no reason to have so much capability for adding html to an img element. This patch eliminates the generalized "Button image HTML attributes" feature and replaces it with a secure alt and class setting.

mlaw’s picture

mlaw commented

Very nice. Patched successfully, the AddThis button validates correctly now.

Thank you! +++

vesapalmu’s picture

vesapalmu commented
Status: Patch (to be ported) » Fixed

I agree that this is a definite bug and it has to be fixed. However this is a change in module functionality and users need to be notified about it. Unfortunately original design if the module caused this bug and there is no safe way of fixing it without changing the functionality.

Patch commited to 6.x dev branch.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.