as per last reports here #1319546: Problem with french accents
and my own observation adding description manually gets double encoded so characters like
ampersand and apostrophe & ' get outputed as ' &

CommentFileSizeAuthor
#23 metatag-n1957358-23.patch4.46 KBDamienMcKenna
#19 metatag-n1957358-19.patch7.61 KBDamienMcKenna
#15 metatag-n1957358-15.patch6.92 KBDamienMcKenna
#14 metatag-n1957358-14.patch3.05 KBDamienMcKenna
#8 description_and-1957358-8.test-only.patch2.53 KBAnonymous (not verified)
#8 interdiff-4-8.txt1.32 KBAnonymous (not verified)
#4 description_and-1957358-4.patch2.07 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Active » Fixed

This problem does not happen in the current -dev version, it correctly handles string encoding.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna commented
Status: Fixed » Closed (fixed)

Now that v7.x-1.0-beta7 is out, I'm closing this in the interest of keeping the issue queue clean.

haysuess’s picture

haysuess CreditAttribution: haysuess commented
Issue summary: View changes

I'm using Metatag beta 9 and Token 1.5.

Any special characters like quotes in my titles or even the body of my content is not properly displayed.

See below (with some properties shortened for easier reading):

<title>How to Change a Person&#039;s Eye Color</title>
<meta property="og:title" content="How to Change a Person&#039;s Eye Color" />
<meta name="description" content="Whether you&#039;re looking to enhance the natural beauty" />
<meta property="og:description" content="Whether you&#039;re looking to enhance the natural beauty" />

You'll see there's a ton of & # 0 3 9 ;

Do I need to upgrade my modules or is there something else going on?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Status: Closed (fixed) » Needs work
FileSize
description_and-1957358-4.patch2.07 KB

I'm reopening this, since I'm still able to reproduce this on a clean D7 install and the metatag HEAD. See attached a test to demonstrate the issue.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review

Thanks for the test. As a reminder, you need to set the issue status to "needs review" to trigger the tests.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Status: Needs review » Needs work

The needs work was for not having a fix yet :)

To clarify:

+++ b/metatag_opengraph/metatag_opengraph.test
@@ -0,0 +1,58 @@
+    // The value in the og:description tag should not be escaped, so this should
+    // fail.
+    $this->assertRaw($page, "bla&#039;ble&amp;blu", 'Metatag');

This should not pass. Actually, we might want to change that to assertNoRaw() to demonstrate the issue better.

I didn't find the time yet to investigate where the double escaping comes from though.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

The test should also query the database to confirm how the data is stored.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Status: Needs work » Needs review
FileSize
interdiff-4-8.txt1.32 KB
description_and-1957358-8.test-only.patch2.53 KB

Status: Needs review » Needs work

The last submitted patch, 8: description_and-1957358-8.test-only.patch, failed testing.

geertvd’s picture

geertvd CreditAttribution: geertvd at XIO commented
Assigned: Unassigned » geertvd

This seems to be quite urgent for us so I'm gonna have a look at this.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

FYI this should be moved to a generic tests/metatag.text_encoding.test file rather than being dependent upon the OpenGraph module.

geertvd’s picture

geertvd CreditAttribution: geertvd at XIO commented
Assigned: geertvd » Unassigned

Actually, after having a closer look, it's seems perfectly normal for these characters to be escaped.
As far as I can see nothing is being double escaped here, the test in #8 also doesn't show that, so for me this is a non-issue, but I'll leave that to @DamienMcKenna to decide.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

I'm looking into this a little more as one of our sites reported the problem too.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review
FileSize
metatag-n1957358-14.patch3.05 KB

I've renamed the class as I suggested above.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: » #2539438: Plan for Metatag 7.x-1.8 release
FileSize
metatag-n1957358-15.patch6.92 KB

I've added another file that simple enables i18n, this will play into #2564483: Cannot translate metatags after last update anymore to make sure that nothing breaks when i18n is enabled.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Fixed

Committed. Thanks!

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Fixed » Needs work

Need to extend this to cover the title tag too. Duh.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review
FileSize
metatag-n1957358-19.patch7.61 KB

I refactored the code so there's just one method that does the testing, and three methods that pass in different types of strings.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Fixed

Committed.

  • DamienMcKenna committed c254b4b on 7.x-1.x 
    Issue #1957358 by DamienMcKenna: Added tests to confirm that HTML...
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Fixed » Needs work

The tests need to be expanded to cover the node's meta tags too, because those are handled different internally than the config values. Gah.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review
FileSize
metatag-n1957358-23.patch4.46 KB

This improves title handling, and adds tests.

  • DamienMcKenna committed f681ab4 on 7.x-1.x 
    Issue #1957358 by DamienMcKenna: Added tests to confirm that HTML...
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Fixed

Committed. Again.

Status: Fixed » Closed (fixed)

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

torsti’s picture

torsti CreditAttribution: torsti as a volunteer commented
Status: Closed (fixed) » Needs work

Hi,

I'm using the lates dev, but title tag still contains characters like:

<title>J&Atilde;&curren;senmaksut</title>

it should be:

<title>Jäsenmaksut</title>

All other Metatag information is outputted ok. My site's default language is Finnish.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

FYI it's completely OK for it to have HTML entities in the title, but it shouldn't be double-encoded :-\

Can you please test the patch in #2564483: Cannot translate metatags after last update anymore to see if that fixes the problem for you?

torsti’s picture

torsti CreditAttribution: torsti as a volunteer commented
Status: Needs work » Fixed

Hi Damien,

Yes, the patch worked - title tag is ok again. Thanks a lot!

Closing this issue again.

Status: Fixed » Closed (fixed)

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