I think it will be more valid, when twitter cards use name instead of property attribute in meta tag.

Works:

<meta property="twitter:card" content="summary">
<meta property="twitter:title" content="My twitter card title">

Valid:

<meta name="twitter:card" content="summary">
<meta name="twitter:title" content="My twitter card title">
CommentFileSizeAuthor
#16 metatag-n2746031-16.patch26.71 KBDamienMcKenna
#16 metatag-n2746031-16.interdiff.txt586 bytesDamienMcKenna
#12 twitter_cards_should-2746031-12.patch26.13 KBcilefen
#12 interdiff.txt3.85 KBcilefen
#11 metatag-n2746031-11.interdiff.txt926 bytesDamienMcKenna
#11 metatag-n2746031-11.patch30.16 KBDamienMcKenna
#9 metatag-n2746031-9.interdiff.txt545 bytesDamienMcKenna
#9 metatag-n2746031-9.patch29.83 KBDamienMcKenna
#8 twitter_cards_should-2746031-8.patch29.92 KBcilefen
#8 interdiff.txt554 bytescilefen
#6 metatag-n2746031-6.patch29.79 KBDamienMcKenna
#4 twitter_cards_should-2746031-4.patch26.13 KBcilefen
#2 name-instead-property_2746031.patch28.13 KBjalpesh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

IT-Cru created an issue. See original summary.

jalpesh’s picture

jalpesh CreditAttribution: jalpesh as a volunteer and at Cybage Software Pvt Ltd. commented
Status: Active » Needs review
FileSize
name-instead-property_2746031.patch28.13 KB

I agree with you because of this twitter guide https://dev.twitter.com/cards/getting-started. A module maintainer would be the best person to comment on this issue. I have attach a patch which will replace property to name.

cilefen’s picture

cilefen CreditAttribution: cilefen commented

The article reads:

Open Graph protocol also specifies the use of property and content attributes for markup () while Twitter cards use name and content. Twitter’s parser will fall back to using property and content, so there is no need to modify existing Open Graph protocol markup if it already exists in your page.

-- https://dev.twitter.com/cards/getting-started

Is this issue necessary?

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue tags: +Needs manual testing
FileSize
twitter_cards_should-2746031-4.patch26.13 KB

It needed a reroll and it works in terms of changing the meta tag "property" to "name". We need this tested on the Twitter cards verifier.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue tags: +govcon2016
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: » #2741339: Plan for Metatag 8.x-1.0-beta10 release
FileSize
metatag-n2746031-6.patch29.79 KB

Adding new tests.

Status: Needs review » Needs work

The last submitted patch, 6: metatag-n2746031-6.patch, failed testing.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Status: Needs work » Needs review
FileSize
interdiff.txt554 bytes
twitter_cards_should-2746031-8.patch29.92 KB
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
metatag-n2746031-9.patch29.83 KB
metatag-n2746031-9.interdiff.txt545 bytes

Just added back the 'node' dependency, for now.

DamienMcKenna’s picture

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

Need to add more tests to confirm the output is correct.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review
FileSize
metatag-n2746031-11.patch30.16 KB
metatag-n2746031-11.interdiff.txt926 bytes

Improving the tests.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
FileSize
interdiff.txt3.85 KB
twitter_cards_should-2746031-12.patch26.13 KB

The tests were made redundant by #2745177: Write tests to ensure all submodules can be enabled & all meta tags are usable.

susannecoates’s picture

susannecoates CreditAttribution: susannecoates as a volunteer commented

Reviewing patch at Drupal GovCon 2016

susannecoates’s picture

susannecoates CreditAttribution: susannecoates as a volunteer commented
Status: Needs review » Needs work
Issue tags: +Needs tests
Related issues: +#2745177: Write tests to ensure all submodules can be enabled & all meta tags are usable

Manual testing seems to indicate that this patch is working. Recommend writing a test for a more thorough/complete evaluation of patch.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Issue tags: -Needs manual testing

Now that #2745177 is in, this needs to be finished off with some tests to ensure the tags work correctly.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review
FileSize
metatag-n2746031-16.interdiff.txt586 bytes
metatag-n2746031-16.patch26.71 KB

Now that we have #2780025: Write tests for confirming all meta tags output correctly committed, this becomes easier to do. This updates the tests.

  • DamienMcKenna committed 796b8da on 8.x-1.x authored by jalpesh 
    Issue #2746031 by jalpesh, cilefen, DamienMcKenna, susannecoates: Fixed...
DamienMcKenna’s picture

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

Committed. Thanks all!

Status: Fixed » Closed (fixed)

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