Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
node system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
2 Jul 2024 at 17:19 UTC
Updated:
31 Jul 2024 at 13:24 UTC
Jump to comment: Most recent
Problem/Motivation
This was added to support RDF module which is now in contrib, we should deprecate then remove it.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3458612
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3458612-remove-legacy-metadata
changes, plain diff MR !8638
Comments
Comment #2
catchComment #4
catchComment #5
longwaveIIRC there are other weird bits of node.module that exist only to support RDF, we should try to remove those as well eventually.
Comment #6
longwaveFixed the CR link, but has this already been removed? Where is
metadataset in $variables anyway? Lots of false positives in grep but I couldn't spot where it is actually set.Comment #7
catchI had the same problem. It's not set anywhere in core, only in RDF module. So we moved RDF out, and just left the support in the templates. I didn't know that twig allows printing completely null variables but apparently it does.
Comment #8
spokjeSeeing that this was for
rdf, would this be the place it was set: https://git.drupalcode.org/project/rdf/-/blob/2.x/rdf.module?ref_type=he...If it is, it was removed from core with the move of rdf to contrib.
Comment #9
catchAnother reason this particular one is crufty is that it only exists because the submitted variable itself is hard-coded in the template. If/when we do #2353867: [META] Expose Title and other base fields in Manage Display the submitted stuff would be in an extra field, and that would give RDF module the mechanism to add metadata directly alongside it anyway.
Comment #10
longwaveLet's add a comment above the deprecation so when we come to this in 12.x we don't have to do the same archaeology to understand it isn't actually used in core.
Comment #11
catchAdded a comment, good plan, it's so confusing.
Comment #12
spokjeDe-confusing comment confirmed, RTBC for me.
Comment #13
catchWe also need to remove this from the six or so copies of node.html.twig around core, probably except for stable9 where it could stay.
Comment #14
longwaveI think
readmoreis also redundant, can't find where that is set in core either, although removing more stuff here is a bit of scope creep, perhaps we should punt this to a separate issue.Comment #15
longwaveIt looks like
readmorewas removed back in #372743: Body and teaser as fields !!!Comment #16
longwaveOh, this is mixed up with #3458782: Remove documentation for readmore, logged_in and is_admin from node.html.twig, the last commit here needs reverting.
Comment #17
catchRemoved the stray commit.
Comment #18
longwaveI realised this wasn't documented in the deprecation policy so opened #3459205: Document how to deprecate Twig variables to fix that.
The change record doesn't say this should be done in preprocess, rather hook_theme, but I guess that doesn't ultimately matter, better to add it close to where it is set? But then the change record should probably be updated to say that too.
Comment #20
ankitv18 commentedI've rebased the branch and removed metadata from profiles and themes.
Pipelines are passing ~~ hence moving into review.
cc: @longwave @catch
Comment #21
catchOpened an issue against RDF module #3462020: $node['metadata'] variable deprecation.
MR looks good to me, removing the novice tag.
Comment #22
longwaveI did a minor rearranging of the code to keep the teaser setup and its deprecation together, otherwise this looks fine to me so RTBC.
Comment #23
catchMinor rearranging looks good!
Comment #26
nod_Committed c1e7cc2 and pushed to 11.x. Thanks!