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

Comments

Status: Needs review » Needs work

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

nicholasthompson’s picture

nicholasthompson
Primary language English
commented
Version: 7.0 » 7.x-dev
StatusFileSize
new 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
commented
Status: Needs work » Needs review

Need review

dixon_’s picture

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

webflo commented
StatusFileSize
new comment-permalink-class-1027936-5.patch730 bytes

Re-roll against 8.x-dev.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
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 commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new comment-permalink-class-1027936-8.patch734 bytes

Re-roll.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
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
commented

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

catch’s picture

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

Committed/pushed to 8.x, thanks!

David_Rothstein’s picture

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
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
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
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.