We need an update for D6 comment.module style signatures. There are a couple of choices we have to make:

Do we want to leave the whole signature issue alone and just let the node signatures module take care of it?
Where we do update, how do we deal with older posts that will have the signature embedded into the code?

CommentFileSizeAuthor
#11 nodecomment-signatures.patch2.44 KBandrewlevine

Comments

merlinofchaos’s picture

merlinofchaos commented
Issue tags: +DruBB
robbiethegeek’s picture

robbiethegeek commented

Ha ha. I just got a request for signatures on the branch with this version of nodecomment for signatures. Can the old school way of grabbing signatures and prepopulating the node form, and ignoring that if signatures for forums is enabled.

merlinofchaos’s picture

merlinofchaos commented

Man, that'll get ugly, I think. It may be possible, but...

merlinofchaos’s picture

merlinofchaos commented

We definitely have an argument that the preprocess functions patch wasn't fully thought through, in terms of how it would affect DX. Perhaps this is my fault, as I was so busy on other stuff that as the only person with real objections to that patch, I didn't feel like standing up and being the guy blocking the patch, and so I put fairly minimal effort into it.

That said, the DX WTF here is mighty, I think, so either a rollback should happen, or this. The problem is, 'this' can only happen if there is a reasonable likelihood of it making it in. The actual patch is likely to be tedious to create.

merlinofchaos’s picture

merlinofchaos commented

Sigh. #9 on wrong issue. I hate how I do that.

andrewlevine’s picture

andrewlevine commented
StatusFileSize
new nodecomment-signatures.patch2.44 KB

Turns out D6 made signatures dynamic. As a result, we can add signatures to node comment relatively easily and let core handle that actual signature creation. Attached is the patch.

zroger’s picture

zroger commented
Status: Active » Needs review

This is definitely the right direction to go with this issue.

I haven't actually tested the patch, but it looks good at first glance.

tom_o_t’s picture

tom_o_t commented
Status: Needs review » Fixed

Committed patch. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -DruBB

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

Issue tags: +DruBB

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