Problem/Motivation

Just upgraded from 1.14 to 1.15. Now the token browser doesn't provide Node tokens any more.

Steps to reproduce

  • Install Metatag 1.15
  • Go to the Metatag content configuration (/admin/config/search/metatag/node)
  • Click on "Browse available tokens"

Expected behavior: The token browser contains node tokens, just like in 1.14
Actual behavior: No node tokens available.

Using node tokens directly still seems to work.

CommentFileSizeAuthor
#15 metatag-missing-token-types-test-only.patch1.35 KBmortim07
#12 metatag-missing-token-types.patch1.95 KBmortim07
#10 metatag-missing-token-types.patch1.79 KBmortim07
#8 metatag-missing-token-types.patch1.65 KBmortim07
#4 metatag-missing-token-types.patch609 bytesmortim07
#2 metatag-missing-token-types.patch623 bytesmortim07
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mrshowerman created an issue. See original summary.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
FileSize
metatag-missing-token-types.patch623 bytes

It appears there are a few missing lines from MetatagDefaultsForm. The variable $token_types isn't being used, unlike in version 8.x-1.14. I've made a restoration patch.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented

With reference to the patch. The metatag.token service should be injected in through the constructor. I'll produce another patch with this change.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
FileSize
metatag-missing-token-types.patch609 bytes

Ah, so it appears it was being injected but never used. I've instead changed the patch to reference this service.

mrshowerman’s picture

mrshowerman
Location Munich
CreditAttribution: mrshowerman at BRANDCODE commented
Status: Active » Needs review

Nice, patch #4 brings back the node tokens. Thanks @mortim07!
Switching to Needs review so that others have a look.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Version: 8.x-1.15 » 8.x-1.x-dev
Status: Needs review » Needs work
Issue tags: +Needs tests

Oh dear, thanks for spotting this and working out some patches.

Any thoughts on adding test coverage for this? I think checking the HTML of the page to confirm the output has the appropriate code to open the token browser would be good.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented

I'll write a functional JS test for it.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
FileSize
metatag-missing-token-types.patch1.65 KB

Added functional JS test.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
Status: Needs work » Needs review
mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
FileSize
metatag-missing-token-types.patch1.79 KB

Woops, forgot docs.

Status: Needs review » Needs work

The last submitted patch, 10: metatag-missing-token-types.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
FileSize
metatag-missing-token-types.patch1.95 KB
DamienMcKenna’s picture

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

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

Would you mind uploading a tests-only patch to confirm the problem? Thank you.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
FileSize
metatag-missing-token-types-test-only.patch1.35 KB

Sure thing!

Status: Needs review » Needs work

The last submitted patch, 15: metatag-missing-token-types-test-only.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mortim07’s picture

mortim07 CreditAttribution: mortim07 at Department of Customer Service, NSW commented
Status: Needs work » Needs review
bwaindwain’s picture

bwaindwain CreditAttribution: bwaindwain as a volunteer commented

patch #12 works good for me

FiNeX’s picture

FiNeX CreditAttribution: FiNeX as a volunteer commented
Issue summary: View changes
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: » #3186720: Plan for Metatag 8.x-1.16

chefkiss.gif

Thank you mortim07! I'll try to get this, and a few other small fixes, out in a new release this week.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Issue summary: View changes

@FiNeX: Please be careful to add your comment to the comment field and avoid erasing the existing issue summary contents.

DamienMcKenna’s picture

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

Committed. Thank you.

FiNeX’s picture

FiNeX CreditAttribution: FiNeX as a volunteer commented

@DamienMcKenna sorry, I've filled the wrong textarea :-)

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles at Morris Animal Foundation commented

Time for a new release to get this into the world?

CulacovPavel’s picture

CulacovPavel CreditAttribution: CulacovPavel commented

This patch is missing in 1.15

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented

The patch wasn't committed before 1.15 was released, it'll be in the new 1.16.

Status: Fixed » Closed (fixed)

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

eiriksm’s picture

eiriksm
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: eiriksm at Ny Media AS, Violinist, Foreningen Drupal Norge commented
Issue tags: -Needs tests

Thanks to everyone involved in fixing the issue, and even adding tests (removed the "needs tests" tag).

Just wanted to politely nudge that more people are interested in a new release including this fix ✌️