Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
http://drupalcode.org/project/xmlsitemap.git/blob/refs/heads/7.x-2.x:/xm...
This uses hook_entity_info_alter() to define its entity. Problem is, token module also uses hook_entity_info_alter() to fill in token types for entities.
So token doesn't get to fill this in for the entity defined here, and so it's missing a token type.
Therefore, it causes modules that assume the token type to always be there if token module is there to show errors: #2197041: PHP notice after clearing the cache: Undefined index: token type in flag_token_info().
Comment | File | Size | Author |
---|---|---|---|
#15 | xmlsitemap_menu_module-2223911-15.patch | 1.01 KB | Grimreaper |
#14 | xmlsitemap_menu_module-2223911-14.patch | 514 bytes | justanothermark |
#7 | xmlsitemap_weight-2223911-7.patch | 946 bytes | Shawn DeArmond |
Comments
Comment #1
Honza Pobořil CreditAttribution: Honza Pobořil commentedSo what solution do you recommend?
Comment #2
joachim CreditAttribution: joachim commentedEither use hook_entity_info() instead of hook_entity_info_alter(), or provide the token type.
Comment #3
DamienMcKennaAs XMLSitemap does not depend upon the Token module, I don't think it's strictly a bug in XMLSitemap.
Comment #4
joachim CreditAttribution: joachim commentedIf you used hook_entity_info() instead of hook_entity_info_alter() then Token module would be able to act and provide the token type for you.
Comment #5
Pafla CreditAttribution: Pafla commentedHow and where to edit this?
Comment #6
dandaman CreditAttribution: dandaman commentedWould it also fix it if we changed xml sitemap to run the alter before token then?
Comment #7
Shawn DeArmond CreditAttribution: Shawn DeArmond commented@dandaman's solution worked for me. I changed the module weight to -1, and it passed all tests.
Attached is a patch that implements it.
PLEASE REVIEW because there might be other consequences of changing module weight.
Comment #8
daveferrara1 CreditAttribution: daveferrara1 commentedHaving the errors too. Mainly triggering this error:
Notice: Undefined index: token type in flag_token_info() (line 88 of /srv/bindings/9fe9a2846c534faab85e3b36c93bb52c/code/sites/all/modules/flag/flag.tokens.inc).
Comment #9
daveferrara1 CreditAttribution: daveferrara1 commentedPatch in #7 worked for me.
Comment #10
GrimreaperHello,
I attach a patch that add a 'token type' key in the hook_entity_info_alter.
I don't test if it is the proper solution but it solved the warning.
I don't think the patch in #7 is a good solution because it only hides the problem. And the hook_update_N in this patch is for a D6, no ?
Thanks for the review.
Comment #11
joachim CreditAttribution: joachim commentedThing is, this is going to happen with any entity info property that another contrib module wants to add to all entities and then rely on.
The real problem is that the hook is being misused: alter hooks should not add a whole new item to an info array.
Comment #12
Dave ReidThe problem is that we're registering an entity type for a table provided by core. What happens when another contrib module wants to declare menu links as entities and does the same thing? *boom* This is why the alter hook is used currently because this is not a theoretical problem at this exact moment.
Comment #13
joachim CreditAttribution: joachim commentedYeah... contrib projects need to agree amongst themselves who gets to entitize a particular core table. Or declare on their project pages that they're mutually incompatible.
Comment #14
justanothermark CreditAttribution: justanothermark commentedI agree this is a wider problem & not sure of the fix but if the workaround in #10 is to be used then it should use the key 'token type' (as mentioned in the comment) and not 'token info' (as used in the patch).
Comment #15
GrimreaperHello,
Thanks for the review, you're right justanothermark, it's 'token type'. I tested at work, and I rewrote a patch at home and I didn't realize I wrote a wrong key.
In the token module in the hook_entity_info_alter :
So the value should be 'menu_link'.
Here is a patch with the right key and value.
Comment #16
thedavidmeister CreditAttribution: thedavidmeister commentedApplying the patch in #15 to the latest dev has caused the notices to cease for me.
Comment #17
kerrycurtain CreditAttribution: kerrycurtain commentedConfirming the patch in #15 worked for me, Thank you.
Comment #18
rooby CreditAttribution: rooby commentedEntitizing (funny word :)) a core table should not be done by some random module that needs to use that data as an entity, because if you try to do it that way then you are relying on some other unrelated module that a user may not want to use.
Ideally it should be in a module that specifically does just the entitizing and then is required by other modules that need it.
So in this case some sort of menu_item_entity module would handle just this entity definition and any other modules that need it require it.
This seems to be kind of what entity_menu_links does although it also has some things that a straight entitizing module doesn't really need, like the UUID/services stuff so it isn't really a candidate for a dependency of xmlsitemap.
Comment #19
joachim CreditAttribution: joachim commented> So in this case some sort of menu_item_entity module would handle just this entity definition and any other modules that need it require it.
Exactly. Same way that a slim File Entity module was refactored out of Media module: https://www.drupal.org/project/file_entity
Comment #20
mamanerd CreditAttribution: mamanerd commentedThe patch in #15 worked for me as well.
Comment #21
Agileware CreditAttribution: Agileware commentedPatch #15 works for me.
Comment #23
Dave ReidCommitted #15 to 7.x-2.x. Thanks everyone.