Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
When _taxonomy_menu_save()
generates the title attribute for the links, doesn't encode possible html special characters originating from the corresponding vocabulary term. This leads in validation failures of type
character "&" is the first character of a delimiter but occurred as data
. The following patch adds some check_plain()
s to prevent this.
Comment | File | Size | Author |
---|---|---|---|
#34 | ampersand.png | 37.74 KB | rkodrupal |
#29 | taxonomy_menu_799428_encoding.patch | 913 bytes | Magur |
#22 | taxonomy_menu_799428_ampersands.patch | 602 bytes | nadavoid |
#21 | taxonomy_menu_799428_ampersands.patch | 524 bytes | nadavoid |
#17 | taxonomy_menu-799428-17.patch | 714 bytes | fengtan |
Comments
Comment #1
Tri CreditAttribution: Tri commentedAttaching patch
Comment #2
indytechcook CreditAttribution: indytechcook commentedThanks! Committed: http://drupal.org/cvs?commit=376086
Comment #3
tomsm CreditAttribution: tomsm commentedI think that this patch generates the following errors in my logs, like:
Details
Type error
Date Friday, 11 June, 2010 - 16:50
User admin
Location http://localhost/drupal/en/admin/content/taxonomy/edit/vocabulary/1
Referrer http://localhost/drupal/en/admin/content/taxonomy/edit/vocabulary/1
Message Menu and taxonomy name mismatch: Fysioband & Tubing != Fysioband & Tubing
Severity notice
Hostname 127.0.0.1
I have these errors since I installed 6.x.-2.8 and rebuilt the menu. If I install the previous version and rebuilt the menu, the errors are gone.
When I remove this patch (check_plain), the errors are gone.
Comment #4
indytechcook CreditAttribution: indytechcook commentedAnybody else confirm this?
Comment #5
jefftrnr CreditAttribution: jefftrnr commentedI think I've confirmed this.
If I have an ampersand in my term name, it displays as & in the menu item when I save the term-edit form.
Comment #6
jefftrnr CreditAttribution: jefftrnr commentedAs a temporary fix, I added a str_replace for each of the 2 elements in the $link array that were changed by the patch. They simply replace & with & again.
It seems fixed now, but there's probably a more elegant solution.
Comment #7
queenbeenz CreditAttribution: queenbeenz commentedI have a similar issue triggered by an apostrophe in the menu.
Comment #8
rogerpfaffI got the issue with an ampersand too
Comment #9
CardinalFang CreditAttribution: CardinalFang commentedSame issue here with apostrophes and ampersands. The problem didn't start until I fooled around with Content Taxonomy and Taxonomy Super Select. Once I disabled and uninstalled those, my problems began.
Comment #10
indytechcook CreditAttribution: indytechcook commentedThe issue occurs on line 1009 of taxonomy_menu.module when the term->name is being compared with the menu item's title. Since the menu item's title hit check_plain() and the term->name didn't it's finding a mismatch. This in no way has any adverse effect on your the menu itself. It's just a warning.
Change line 1009 from
To
I'm not familiar enough with the tt function to know it should also be wrapped to check_plain. I tend to think it should as those lines of code overwrite the original check_plain.
I'll make a proper commit later today. Please test and let me know if this works.
Comment #11
indytechcook CreditAttribution: indytechcook commentedHere is a patch against the latest from the DRUPAL-6--2 branch. Since my i18n usage is nill, i'd love it if someone would test this.
Comment #12
indytechcook CreditAttribution: indytechcook commentedAdded comment from #841440: Check plain transforms HTML tags instead of removing them by doitDave:
This will be committed with the above patch when the patch has been tested.
Comment #13
sreese CreditAttribution: sreese commentedI have tried the patches posted in this thread and rebuilt my taxonomy menus but to no avail. I still see '& amp;' (had to add a space there) instead of '&' in the menu. Is there some trick to getting this to work that I am unaware of?
Comment #14
happysnowmantech CreditAttribution: happysnowmantech commentedI am seeing the problem with ampersands not being displayed properly as well, e.g. if I have a term named 'A & B', the menu displays 'A & B'
FYI, theme_menu_item_link() also runs the menu item title through check_plain(), via its call to l(). If the title is run through check_plain() twice, once in _taxonomy_menu_save() and then again in theme_menu_item_link(), the & problem will result.
For now, I'm using a patch similar to #6 as a temporary workaround:
Comment #15
sreese CreditAttribution: sreese commentedThe temporary workaround posted in comment #6 also worked for me. Had a bit of a hard time figuring out where to put it but ended up adding the 3 lines starting at line 554 in the -dev version of taxonomy_menu.module which is towards the end of the _taxonomy_menu_save($item) function.
Comment #16
giorgio79 CreditAttribution: giorgio79 commented+1
& in term names turned to &
Comment #17
fengtanHere is another patch to get rid of the
&
but also of any html entities.I don't think performing a
check_plain()
is needed on$item['name']
since$item
is to be saved withmenu_link_save($item)
, which does not expect$item['name']
to be sanitized.Comment #18
indytechcook CreditAttribution: indytechcook commentedHere is the commit: http://drupal.org/cvs?commit=391318
Please test this when the new dev release is rolled in the next 12 hours.
Comment #19
nadavoid CreditAttribution: nadavoid commentedI'd like to test, but upon enabling the latest dev, I get this error:
Comment #20
tomsm CreditAttribution: tomsm commented@ nadavoid
An issue has been opened about this error:
http://drupal.org/node/853438
Comment #21
nadavoid CreditAttribution: nadavoid commented@tomsms I tried the latest dev which looked like it had #853438: Fatal Error - Can's use function return value in write context applied. Thanks for pointing that out. It works, and I am able to test further now.
Now, when entering new terms, the description of the term (which is used as the title attribute of the menu item) displays html encoded ampersands. I have a wysiwyg installed. When a user enters an ampersand, it is html encoded by the wysiwyg. That is presented exactly as entered by the menu system. However, this is not what anyone would want. So I'm submitting a patch that replaces the html encoded ampersand with a single ampersand character.
Attaching a patch which replaces this:
with this:
The check_plain is not needed because the menu item rendering is run through check_plain. With check_plain here, we get double encoded html entities rendered in the title.
The html_entity_decode is just a way to decode any html-encoded characters. I currently have some encoded ampersands because I have a wysiwyg enabled for term descriptions.
I'd appreciate any feedback. Thanks!
Comment #22
nadavoid CreditAttribution: nadavoid commentedUpdated patch. Same update applying to term title if term description is empty.
Comment #23
hanoiisubscribe
Comment #24
indytechcook CreditAttribution: indytechcook commentedOk. Let's hope this is the last change because I want to do a new release.
http://drupal.org/cvs?commit=396554
PS. I really need to spend time to make some tests.
Comment #25
nadavoid CreditAttribution: nadavoid commentedI just tested the latest dev release which has this patch applied, and it works as expected for me. Marking RTBC. Thanks for committing it indytechcook!
Comment #26
tomsm CreditAttribution: tomsm commentedI also tested latest dev. All ok.
Comment #27
indytechcook CreditAttribution: indytechcook commentedSetting to fixed with the new release: http://drupal.org/node/872158.
Comment #29
Magur CreditAttribution: Magur commentedIf I have special characters in the description of the term like "Jahresangabe für Berichte" then i get a database entry in the table menu-links which has the following content in the options-field:
So everything after (and including) the special character is missing.
I think the reason for this is that html_entity_decode decodes a character to ISO-8859-1 by default. Setting it to UTF-8 fixes this issue for me.
Comment #30
indytechcook CreditAttribution: indytechcook commentedNeed someone else to test this.
Comment #31
wuh CreditAttribution: wuh commentedThe patch doesn't work for me - I still have menu titles with unencoded html special chars - I use the i18n module - could this be causing issues?
Comment #32
organicwire CreditAttribution: organicwire commentedI had the same problem as I had a taxonomy term having an ampersand in it's name. The problem was resolved updating to the new module version (6.x-2.9) AND opening+saving the taxonomy term afterwards.
Comment #33
abx CreditAttribution: abx commentedJust tried version 2.9 and I can't get menu list of the taxonomy name with "slash" in it. :(
plaque de cuisson
Comment #34
rkodrupal CreditAttribution: rkodrupal commentedSorry if this isn't the correct discussion thread, but I'm still getting my ampersands showing up as unencoded for taxonomy terms ... see attachment.
I get this behavior for both new terms and for old terms that I've updated.
addendum: I am at 6x-2.9 on Taxonomy Menu
addendum#2: firebug shows the "&" as "&" ... and I'm using bb2html (http://drupal.org/node/28537, which doesn't seem to be getting invoked)
Comment #35
rkodrupal CreditAttribution: rkodrupal commentedupdate on #34 ... bb2html does indeed get invoked. I can place [amp] in the taxonomy item and it shows up okay as an ampersand (due to the magic of bb2html) ... but bb2html doesn't work everywhere and [amp] just shows up as [amp] ...
Comment #36
rkodrupal CreditAttribution: rkodrupal commentedI found a fix for me and am sharing it so it may help others:
replaced $title with strip_tags(html_entity_decode($title)) line 205 in page.tpl.php
... this inspired me to also do the following:
replaced $snippet with strip_tags(html_entity_decode($snippet)) in search-results.tpl.php
I use the zincout theme, so you may need to find the equivalent tpl's for your theme.
Comment #37
Agileware CreditAttribution: Agileware commentedI have been having problems when my term description includes
The same as those described in #29.
The patch in #29 fixes the problem for me.
I am not using i18n.
The problem was causing this error:
because when drupal unserializes the array
You get FALSE.
Comment #38
Agileware CreditAttribution: Agileware commented@abx re #33:
It is not a good idea to change old posts because it is hard for people to see what has changed.
If you have just made the post it's fine but not if it is months old because people expect the comments to be in chronological order and you will find anything that gets edited back into an old post is likely to be ignored because people don't see it.
Comment #39
abx CreditAttribution: abx commented@Agileware
That was strange but that's not me who changed it. I didn't come back to this topic since I replied on that one. (And it's not Jan 30, 2011 for sure. I think I replied it sometime in the last quarter of 2010) Still, wondering if it's problem on drupal.org or my own computer.
Could it be some security problem of Drupal.org? I'm quite sure I was not the one that put up the link "plaque de cuisson" on it. I believe it's done by some spam robot.
Comment #40
soulfroys#29 fix it for me! (6.29)
Comment #41
dstolComment #48
dstolThe Drupal 6 version is no longer supported