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.
Problem/Motivation
Token descriptions and token type descriptions should only ever be used if titles cannot hold sufficient information. They're an optional addition, and should never be a requirement.
Proposed resolution
This is primarily a policy decision. Patch updates the the descriptions to list them as optional and removes the description on a few example descriptions eg. node title.
Remaining tasks
n/a
User interface changes
n/a
API changes
Change notice required
Original report by xano.
Descriptions should only ever be used if titles cannot hold sufficient information. They're an optional addition, and should never be a requirement.
Comment | File | Size | Author |
---|---|---|---|
#11 | drupal_1911768_11.patch | 3.2 KB | Xano |
Comments
Comment #1
XanoComment #2
Xano#1: drupal_1911768_00.patch queued for re-testing.
Comment #4
Xano#1: drupal_1911768_00.patch queued for re-testing.
Comment #5
Dave ReidLet's just actually make them optional rather than providing defaults for them. It's pointless to merge in NULL values.
Comment #6
XanoIt prevents us from having to add isset() everywhere and gives us a behavior that is more similar to using classes, without actually having to convert the entire system.
Comment #7
Dave ReidThat's not a pattern that is used in Drupal core's info hooks at all for truly optional values though. I don't think we should start here.
Comment #8
XanoIt does for annotations, for instance. We are clearly moving to a situation in which we provide default values for optional information.
Comment #9
XanoComment #10
XanoBump.
Comment #11
XanoI removed some redundant node token descriptions as a proof of concept.
Comment #12
Xano11: drupal_1911768_11.patch queued for re-testing.
Comment #13
jsibley CreditAttribution: jsibley commentedIssue 2159989 for Drupal 7 was pointed to this issue. Yet, as pointed out in that issue, this appears to be for Drupal 8.
Is this fix supposed to address issues in Drupal 7, as well, once Drupal 8 is fixed. Will it be backported and will that status be reported as part of this issue?
Thanks.
Comment #14
XanoMaybe, depending on how much it would break backwards compatibility. That discussion will not be started until this has first been fixed for Drupal 8, though.
Comment #15
Xano1: drupal_1911768_00.patch queued for re-testing.
Comment #17
Xano11: drupal_1911768_11.patch queued for re-testing.
Comment #18
heddnComment #19
heddnComment #20
heddnReviewed the code. It makes sense. There seems to be general consensus on the recommended approach. Marking RTBC.
Comment #21
Dave ReidThe patch in #11 doesn't actually include the merging of defaults that was in patch #1.
Comment #22
Dave ReidOk, confirmed with @Xano today that not merging in default is actually the desired behavior, so confirmed this is RTBC. I'm glad we're getting rid of the completely unhelpful descriptions that are just duplicates of the token name itself. Should we have a follow-up issue to review all the existing tokens for redundant descriptions?
I don't think this should be backported to Drupal 7 however.
Comment #24
alexpottCommitted f9dc1f1 and pushed to 8.x. Thanks!