In comment.module, there is a preprocess function which incorrectly sets the class of a link as a string. Should be an array. This was exposed when I wrote a custom module to handle comment_uri differently.

The attached patch fixes the bug by wrapping the "permalink" into an array.

CommentFileSizeAuthor
#8 comment-permalink-class-1027936-8.patch734 byteswebflo
#5 comment-permalink-class-1027936-5.patch730 byteswebflo
#2 comment.module.preprocess_bug.patch880 bytesnicholasThompson
comment.module.preprocess_bug.patch838 bytesnicholasThompson
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, comment.module.preprocess_bug.patch, failed testing.

nicholasThompson’s picture

nicholasThompson
Primary language English
CreditAttribution: nicholasThompson commented
Version: 7.0 » 7.x-dev
FileSize
comment.module.preprocess_bug.patch880 bytes

Ok - trying again against HEAD, at it appears there is no DRUPAL-7--1 branch yet... and the bug still exists in HEAD.

nicholasThompson’s picture

nicholasThompson
Primary language English
CreditAttribution: nicholasThompson commented
Status: Needs work » Needs review

Need review

dixon_’s picture

dixon_
he/him
Primary language English
Location France
CreditAttribution: dixon_ commented
Version: 7.x-dev » 8.x-dev
webflo’s picture

webflo CreditAttribution: webflo commented
FileSize
comment-permalink-class-1027936-5.patch730 bytes

Re-roll against 8.x-dev.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

5: comment-permalink-class-1027936-5.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 5: comment-permalink-class-1027936-5.patch, failed testing.

webflo’s picture

webflo CreditAttribution: webflo commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
comment-permalink-class-1027936-8.patch734 bytes

Re-roll.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Status: Needs review » Reviewed & tested by the community

I'd almost award the trivial patch of the month award :)

RTBC!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

8: comment-permalink-class-1027936-8.patch queued for re-testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +Needs backport to D7

Needs backport, but there's more discussion at #1200478: Changing the comment path to the node path triggers a PHP fatal error in l() due to wrongly structured options (which also considered tests).

One or the other issue should probably be closed.

pounard’s picture

pounard
Primary language French
CreditAttribution: pounard commented

Note that there is working patches there! All there is to do is to grab and commit them (might need reroll though).

pounard’s picture

pounard
Primary language French
CreditAttribution: pounard commented

Issue is opened there since 2011 and I'm personally experiencing it on two projects since 2012, it's more than ever time to commit this fix, it's sad being forced to use a patch on all projects for such a trivial fix and old bug.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Fixed
Issue tags: -Needs backport to D7

Let's go with #1200478: Changing the comment path to the node path triggers a PHP fatal error in l() due to wrongly structured options.

Status: Fixed » Closed (fixed)

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