Does not work with Nodecomment module

I tried the Node Comment module and have had nothing but problems using it.
With a clean install of Drupal and using Garland I keep getting the following error message:

Fatal error: Unsupported operand types in /var/www/test.org/includes/common.inc on line 1445

Perhaps you should file a bug report in the Node Comment issue queue.

Jay,

I have used nodecomment for the last 6 months with D6 without any issues. I have never seen the error you're getting.

After taking a look at the theme code in template.php, it seems the issue lies in phptemplate_preprocess_node().

@pgp78 - OK. I'll bite. Where in phptemplate_preprocess_node() is the problem?

Wow, that was a while back... Hopefully you still would be willing to give it a try.

I found the same problem and dug out this bug when looking for a way to use acquia_marina+nodecomment+advanced_forum.

Here is the offending code, starting on line 295:

...
  if (isset($vars['node']->links['comment_add'])) {
    $node_content_type = (theme_get_setting('comment_enable_content_type') == 1) ? $vars['node']->type : 'default';
    if ($vars['teaser']) {
      $vars['node']->links['comment_add'] = array(
        'title' => acquia_marina_themesettings_link(
          theme_get_setting('comment_add_prefix_'. $node_content_type),
          theme_get_setting('comment_add_suffix_'. $node_content_type),
          t(theme_get_setting('comment_add_'. $node_content_type)),
          "comment/reply/".$vars['node']->nid,
          array(
            'attributes' => array('title' => t(theme_get_setting('comment_add_title_'. $node_content_type))), 
            'query' => NULL, 'fragment' => 'comment-form', 'absolute' => FALSE, 'html' => TRUE
          )
        ),
        'attributes' => array('class' => 'comment-add-item'),
        'html' => TRUE,
      );
    }
    else {
      $vars['node']->links['comment_add'] = array(
        'title' => acquia_marina_themesettings_link(
          theme_get_setting('comment_node_prefix_'. $node_content_type),
          theme_get_setting('comment_node_suffix_'. $node_content_type),
          t(theme_get_setting('comment_node_'. $node_content_type)),
          "comment/reply/".$vars['node']->nid,
          array(
            'attributes' => array('title' => t(theme_get_setting('comment_node_title_'. $node_content_type))), 
            'query' => NULL, 'fragment' => 'comment-form', 'absolute' => FALSE, 'html' => TRUE
          )
        ),
        'attributes' => array('class' => 'comment-node-item'),
        'html' => TRUE,
      );
    }

I don't fully understand the intent of completely rewriting the link. I think the code should preserve all elements as is and only mod the relevant parts. In case of nodecomment, using hard-coded "comment/reply/".$vars['node']->nid, seems to be the root cause of the problem.

I think I know how to fix this problem, and will submit a patch soon.

I'm going to bump this up to critical. This is a very popular theme and I'm hoping to have most if not all of my 6K users to nodecomment within the next few months. There's going to be a ton of complaints if this isn't fixed by then.

This whole overwriting of the will likely also cause problems with the AF buttons and changing the text to "reply" but that's minor compared to it linking to completely the wrong place.

I don't understand what it's doing enough to provide a complete patch, but I suggest at least replacing the hardcoded parts with the values that are already in the $links variable.

Michelle

First I want to post some analysis of the above code in question.

- It transforms a regular link into a pre-parsed HTML.

Incoming link:

Array (
    [title] => Add new Comment
    [attributes] => Array (
            [title] => Add a new comment to this page.
        )
    [href] => node/add/comment/2
)

Data after the transformation:

Array (
    [title] => <a href="/comment/reply/2#comment-form" title="Share your thoughts and opinions related to this posting.">Add new comment</a>
    [attributes] => Array (
            [class] => comment-node-item
        )

    [html] => 1
)

The final HTML in 'title' element is created by acquia_marina_themesettings_link() function:

function acquia_marina_themesettings_link($prefix, $suffix, $text, $path, $options) {
  return $prefix . (($text) ? l($text, $path, $options) : '') . $suffix;
}

The apparent reason of doing it is to add prefix and suffix to the link. I have to question the value of having prefix and suffix, but it is for a different thread. I agree with Michelle - at minimum, the code should use href from incoming data, not shove in a hard-coded value. The fix is simple, but there is somewhat tricky part - to make sure that other elements of original link are passed in properly, like 'fragment', default 'title', etc.

One more analysis point - this code pattern is used on other links as well, and may cause problems in the same way - when some module needs to change the href, acquia_marina will interfere. Therefore I will craft a single patch that addresses all these places at once.

Gotta go - I will work on that later.

The 6.x-2 branch is the future of this theme.
Please verify that this is still an issue in the 2.x branch and patch against that branch.
Thank you.

Here's the patch, rolled against acquia_marina 6.x-2.x-dev (2009-Jun-13).

Please review for commit into 2.x-dev.

It fixes all links href and fragment being overwritten by hardcoded values.

I believe the code you're referring to is for the theme settings, so the text for the read more/add comment/comments/etc links can be updated on a per content type basis in the theme settings. Can you confirm that this patch works with those theme settings?

I really disagree that this is a feature request. By hardcoding URLs instead of using the ones provided in the variable, you are breaking normal functionality, which would be a bug. So I do hope you commit this, assuming the theme settings can be confirmed to work. I haven't tested the patch, yet, but I looked at the code and the idea behind it seems sound.

Michelle

@stephthegeek
Thanks for looking into this.

I confirm that this patch works with all theme settings. All this patch does is makes sure that when theme settings are applied, the URLs of the links it changes are preserved. I paid special attention to not change any of the intended effects, only to properly copy URL parts into the new link.

The code in question is indeed for theme settings, but the problem has nothing to do with the text of the links, but with URLs. What current AM 2.x code does is uses an assumption of where the links should point, and this screws up URLs of the links, so they point to completely the wrong places. It introduces one nasty effect, for instance - comments posted using the "Add new comment" link that this code massaged do not show up. To verify there is a problem, you will have to install and enable nodecomment module, or any other module that uses different location for comments. For example, nodecomment works with Garland, but regresses under acquia_marina. That falls under a definition of a bug in my understanding. Also, it is critical regression that prevents use of acquia_marina+nodecomment on a production site.

Can someone from a community also verify this patch and post results here?

Ok, I tested and confirmed that the patch does fix the links. I also can still use the theme settings to change the title on the "add comment" link on the node. It doesn't have any effect on nodecomment nodes but that probably is due to nodecomment itself rather than this patch.

Looks like there's going to be a lot of other work needed to get AM to work with AF, but this at least gets it to work with NC so it's a good start.

As an OT, these settings seem to be going way above and beyond what I would consider the job of a theme. Have you considered seperating this out into a module? By making this a theme setting, you clobber whatever modules do to links rather than working with them on the preprocess chain.

Michelle

@iva2k - Thank you for the patch! We will review and commit shortly.

Hi there,

I'm relatively new to Drupal, I'm very technical, but not a programmer. I have tried to apply the patch and all I get when I put up the new file is a blank white screen, no errors, no messages, nothing. I believe the patch has been applied successfully, and was just wondering if anyone had any ideas for me to try?

If I can get the patch working I would be eternally greatful, I absolutely love this theme, but absolutely need to run nodecomment.

Many thanks for your time.
~MattMc

We're in the process of porting the Acquia themes over to our new theme system, Fusion.
With the release of Fusion, we will no longer have theme settings which duplicate the functionality of contrib modules.
We're unable to dedicate resources to fixing this since Fusion will solve this issue.

Thanks for all those who put effort into resolving this.

You can read more about Fusion at:

TNT 2.0 and the future of Drupal themes with Fusion
http://www.topnotchthemes.com/blog/090709/tnt-2-0-and-future-drupal-them...

Top Notch Themes pre-releases a landmark theme called Fusion
http://chrisshattuck.com/blog/top-notch-themes-pre-releases-landmark-the...

That makes sense. Thanks for letting us know, jwolf.

Michelle

I have been following this thread because I some how think a have the same problem.

What happends is that the reply link in the top and buttom of the page and also the "Add new comment" all goes to the address (this is just an example):

http://www.moowbi.com/comment/reply/146#comment-form

Where as the reply button inside the comments go to the address:

http://www.moowbi.com/comment/reply/146/16

The last one mentioned works fine. But the first one opens a form whith out a "save" button and then tells that I have to be a member of the group to post it. Even if I AM a member of the group. It seems like it gets out of the group context and for that reason can't relate the user to the group.

If I log in as administrator the problem is gone. I guess this is because an administrator don't need the relation but actually can do almost anything.

At first I thought that it was about permissions. But I granted all possible permissions whithout result. And when I read this thread I saw the posibility that this might be related to my problem.

I have applyed the patch from this thread but whithout result.

I have struggled with this for weeks now and I just can't find a way out.

Does any one of you have an idea about whats going on?

(Running: Drupal 6.14, advanced_forum-6.x-1.x-dev, og_forum-6.x-2.2 and I am using the acquia_marina theme)

comment to my own comment at #19:

I apologise for the disturbance in this thread as I found out that my problem mostly was based on my own lack of basic Drupal knowledge. I just had to activate the Node comment in the content types. Thats it. It was though followed by other issues like:

The Reply buttons are now "Locked" (but the "add new comment" works as intended)
The theming from AF is gone.

Any feedback on those?

Since the original problem remains in 6.x-2.0 release, I rerolled the patch for latest 6.x-2.0-dev. Please review.

Better patch - also covers the case when Acquia Marina removes links already rendered by other modules (it happens for Advance Forums).

2.x is no longer supported