Non-field managed files do not have descriptions. It's not in the schema at all. But somehow we have a [file:description] token which causes a PHP notice when it is used (I started implementing a [user:picture] chained token in D7).

CommentFileSizeAuthor
#13 978028-file-description-token-no-more.patch3.89 KBmuriqui
#10 978028-file-description-token-no-more-D7.patch3.89 KBmuriqui
#5 978028-file-description-token-no-more-D7.patch3.7 KBDave Reid
#4 978028-file-description-token-no-more-D7.patch3.68 KBDave Reid
#2 978028-file-description-token-no-more-D7.patch2.96 KBDave Reid
#1 978028-file-description-token-no-more-D7.patch2.54 KBDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Active » Needs review
FileSize
978028-file-description-token-no-more-D7.patch2.54 KB

On my dear lord. The File token tests manually add $file->description = ... in order to pass. So, so, so wrong.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
978028-file-description-token-no-more-D7.patch2.96 KB

Forgot to remove it from system_token_info as well.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Title: Simple managed files do not have descriptions (PHP notice with [file:description] » Managed files do not have descriptions (PHP notice with the [file:description] token)
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
978028-file-description-token-no-more-D7.patch3.68 KB

Without reverting a potential security hole this time.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
978028-file-description-token-no-more-D7.patch3.7 KB
joachim’s picture

joachim CreditAttribution: joachim commented

What if you want the description on managed files?

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

Then you want *field* tokens as provided by Token.module for D7 because the description is field instance data. The file tokens should only work with a proper file object from file_load().

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

@joachim Feel like RTBCing this one now that it makes sense?

muriqui’s picture

muriqui CreditAttribution: muriqui commented

#5: 978028-file-description-token-no-more-D7.patch queued for re-testing.

muriqui’s picture

muriqui CreditAttribution: muriqui commented
FileSize
978028-file-description-token-no-more-D7.patch3.89 KB

Rerolled patch to work with latest 7.x.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

@muriqui: Please attach the patch again without the '-D7' on the end of it so it can be tested properly.

Status: Needs review » Needs work

The last submitted patch, 978028-file-description-token-no-more-D7.patch, failed testing.

muriqui’s picture

muriqui CreditAttribution: muriqui commented
Status: Needs work » Needs review
FileSize
978028-file-description-token-no-more.patch3.89 KB

Whoops.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7
Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

#13: 978028-file-description-token-no-more.patch queued for re-testing.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Needs review » Reviewed & tested by the community

Looks good. Let's fix this now so we stop people from trying to use an invalid token.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs review

Can we get at least one review on this? Since muriqui just re-rolled your patch, you've essentially marked your own patch RTBC here.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Title: Managed files do not have descriptions (PHP notice with the [file:description] token) » File token fixes: [file:description] does not exist and [file:owner] not using format_username
joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs review » Reviewed & tested by the community

Patch applies and looks good.

muriqui’s picture

muriqui CreditAttribution: muriqui commented

@webchick Does my review count, or do we need someone else since I was involved with the re-roll?

muriqui’s picture

muriqui CreditAttribution: muriqui commented

Never mind, looks like joachim marked it RTBC before I did.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x and 7.x. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -token, -Needs backport to D7

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

Issue tags: +token, +Needs backport to D7

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.