Chameleon relies on theme_blocks, which was removed from includes/theme.inc by #351235: hook_page_alter()

CommentFileSizeAuthor
#7 chameleon_frh.patch3.36 KBmr.baileys
#5 374650-chameleon.patch3.14 KBmr.baileys
#4 chameleon_frh.patch3.36 KBFrando
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Frando’s picture

Frando CreditAttribution: Frando commented

so.. let's just remove chameleon?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Hm. PHP themes are still a valid means of designing a Drupal site. IMO we need to put this ability back, even if Chameleon is ultimately removed from core.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: Chameleon broken by #351235 » Chameleon broken by hook_page_alter

Very few people actually have issue IDs memorized, so changing title. ;)

Frando’s picture

Frando CreditAttribution: Frando commented
Status: Active » Needs review
FileSize
chameleon_frh.patch3.36 KB

OK. Patch attached. Node pages and listings were completely broken as well, so I also fixed chameleon_node while I was at it.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys commented
Status: Needs review » Reviewed & tested by the community
FileSize
374650-chameleon.patch3.14 KB

Thanks Frando,

checked and applied patch and works like a charm. My only gripe would be readability: I'd switch the "render comments" and "Render remaining parts of the node content.", because right now the order is:

  1. Render comments
  2. Add content to output
  3. Add comments to output
@@ -152,11 +152,20 @@ function chameleon_node($node, $teaser = 0) {
     $output .= '<div class="links">' . $terms . $links . $submitted . "</div>\n";
   }
 
+  // Render comments.
+  if (!empty($node->content['comments'])) {
+    $comments = drupal_render($node->content['comments']);
+  }
+  else {
+    $comments = '';
+  }
+
+  // Render remaining parts of the node content.
+  $output .= drupal_render($node->content);
+
   $output .= "</div>\n";
 
-  if ($node->content['comments']) {
-    $output .= drupal_render($node->content['comments']);
-  }
+  $output .= $comments;
 
   return $output;
 }

Patch attached just moves the "render comments" logic down a bit. If this is ok with you, I think this is RTBC.

Frando’s picture

Frando CreditAttribution: Frando commented
Status: Reviewed & tested by the community » Needs work

I haven't tested your patch but I don't think it works as intended. Placing drupal_render($node->content['comments']) above drupal_render($node->content) was on purpose, because drupal_render($node->content) will render everything in $node->content that hasn't been rendered so far, including comments, if they haven't been rendered before.

We want comments to appear below e.g. upload attachments, so we have to first render the comments, put them into a variable, then render the remaining content parts, and then append the rendered comments to the output. This is what my patch does.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys commented
Status: Needs work » Needs review
FileSize
chameleon_frh.patch3.36 KB

Strange, changing the order does seem to work (looks like $node->content['comments'] is rendered earlier?)

In any case I can follow your reasoning as to why the order shouldn't be changed. I'm going to re-attach your original patch and put this back to CNR. I'd set it to RTBC, but I feel my previous post caused me to lose that privilege ;).

Dries’s picture

Dries CreditAttribution: Dries commented

Let's remove Chameleon instead? :)

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

Ok. I volunteer to take Chameleon and the other crufty themes and maintain them in contrib.

There’s some project/CVS permissions problems for me with bluemarine and pushbutton since they already exist in contributions/themes.

But I've created http://drupal.org/project/chameleon. Also, mr.baileys and frando’s patch works as advertised which I've committed to contrib.

Subscribing for now; I need to head to the airport all too soon.

jrabeemer’s picture

jrabeemer CreditAttribution: jrabeemer commented

+1 vote to remove Chameleon theme. I just enabled it for kicks and was surprised to get a white screen of death.

I think we need to remove ALL the old crufty themes.

As a tangent, I was at DrupalCon DC BoF with designers. Those guys are chomping at the bit to do a theming/design competition to create new Drupal themes. Opportunity waiting?!?!?!

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys commented
Status: Needs review » Fixed

@John: thanks for volunteering to maintain Chameleon in contrib. As Frando's patch has been commited, this issue can be marked fixed.

Actual removal of Chameleon from core can be addressed in #315533: Remove all themes but Garland and Stark from core.

Status: Fixed » Closed (fixed)

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