attribute "class" is being outputted twice in the node.tpl.php file of wetkit_omega.

line 90:

<article id="node-<?php print $node->nid; ?>" class="<?php print $classes; ?> clearfix"<?php print $attributes; ?>>

class is being manually added by <?php print $classes; ?> , "clearfix" being appended and then <?php print $attributes; ?>add the same class

I would suggest keeping only print attributes since it includes other attributes like role="article" and adding clearfix to the attributes class array.

Also, I noticed the same issue happening on many other .tpl files in wetkit_omega theme.

For example:

  • comment.tpl.php
  • comment-wrapper.tpl.php
  • taxonomy-term.tpl.php
  • block--views--headlines-front-page-block.tpl.php
  • block--sidebar_first.tpl.php
  • etc...

Any reason for this markup specific to wetkit_omega?

CommentFileSizeAuthor
#9 revert-commit-62064fb_2232467-9.patch9.8 KBAnonymous (not verified)
#8 outputting-class-twices-8892453-8.patch1.24 KBAnonymous (not verified)
#6 outputting-class-twices-8892453-6.patch2.31 KBAnonymous (not verified)

Comments

sylus’s picture

sylus commented
Priority: Normal » Critical

Nope no reason just likely an issue or a screw up.

Lets fix this for v1.4.

Updating to critical as will affect accessibility.

sylus’s picture

sylus commented

Can you supply a patch for these issues? Would be super grateful :)

sylus’s picture

sylus commented
Status: Active » Fixed

Commit: http://drupalcode.org/project/wetkit_omega.git/commit/62064fb

Did my best to address all the issues I could find. Can open individual issues for anything missed.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Anonymous (not verified) commented

I see a potential side effect with the commit from comment #3, the `clearfix` class have been removed from `comment.tpl.php` and `node.tpl.php` which some design might rely on.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new outputting-class-twices-8892453-6.patch2.31 KB

If we look at the pattern that was replaced (printing both `$classes` and `$attributes`), it is definitely not a markup specific to wetkit_omega, it's there in 7 files from 6 core modules, and 70 files from 7 contrib modules wetkit is using. I don't think we should change all these files to please Omega. The real problem lies in `omega/template.php` function `omega_theme_registry_alter` where they copy the content of `classes_array` into `attributes_array['class']` with the following comment:

// We prefer the attributes array instead of the plain classes array used by
// many core and contrib modules. In Drupal 8, we are going to convert all
// occurrences of that into an attributes object. For now, we simply
// synchronize our attributes array with the classes array to encourage
// themers to use it.

I suggest that we rollback the commit from comment #3 and edit wetkit_omega to revert the hack Omega is doing, this will then play nice with the whole drupal ecosystem, as it will be done "the drupal (7) way".

I know this is "not playing nice" with Omega, but it's a tough decision between betraying Omega, or the whole Drupal 7 ecosystem :-) I am definitely open to hear a better solution, but meanwhile, I will use this patch for my project.

Anonymous’s picture

Anonymous (not verified) commented
Status: Closed (fixed) » Needs work
Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new outputting-class-twices-8892453-8.patch1.24 KB

patch updated, as there was a weird copy/paste (or distraction) error in there

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new revert-commit-62064fb_2232467-9.patch9.8 KB

and here's a patch to revert commit #62064fb

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs work » Reviewed & tested by the community

Patches pass testing in our environment (we tested these two patches against the latest wetkit_omega module). The patched wetkit_omega no longer has the double class problem.
Please commit patches from comment #8 and #9 to the wetkit_omega project asap.

My git account is still bombed, so someone else please commit the patch

gdaw’s picture

gdaw commented

Is the patch still needed for this?

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

Requesting co-maintainer access on the 1.x branch.

sylus’s picture

sylus commented
Status: Reviewed & tested by the community » Fixed

Committed and attributed!

  • sylus authored 549c932 on 7.x-1.x 
    Update WetKit Omega for Issue #2232467 by eleclerc | ptsimard: Fixed...
sylus’s picture

sylus commented
Status: Fixed » Needs work

Had to revert this commit as the hook_registry_alter is causing a lot of warnings throughout the site.

Warnings such as:

Notice: Undefined index: class in omega_preprocess_page() (line 13 of /mnt/www/html/wet-boew-drupal-7.x-1.x/profiles/wetkit/themes/omega/omega/preprocess/page.preprocess.inc).
Warning: array_diff(): Argument #1 is not an array in omega_preprocess_page() (line 13 of /mnt/www/html/wet-boew-drupal-7.x-1.x/profiles/wetkit/themes/omega/omega/preprocess/page.preprocess.inc).

This is because of omega itself using the new class variable and it no longer be set thanks to the hook_registry_alter

$variables['attributes_array']['class'] = array_diff($variables['attributes_array']['class'], array(drupal_html_class($hook)));

I think it might be too much work as this is how Omega 4.x did this and I don't like the idea of us manually fixing all of Omega's preprocess functions ourself. Is the only thing missing from earlier commits a clearfix logic needing to be applied to comments etc? Even though how Omega does it is a departure from core it should still work based on my first set of commits above. We can leave the 2.x branch and bootstrap to call it the default way which it does.

  • sylus committed 3d6758d on 7.x-1.x 
    Update WetKit Omega for Revert "Issue #2232467 by eleclerc | ptsimard:...
joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs work » Needs review

*EDIT* it would be nice to fix this to stabilize the 1.x branch for those that will still be using it.

I looked at the reverted commit to wetkit_omega but didn't get to test it yet. a cache clear should normally flush out tpl files which were changed. When spare cycles retest.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs review » Needs work
sylus’s picture

sylus commented
Status: Needs work » Fixed

After talking with Eric this should actually be all fixed he was only concerned about the clearfix being missing since it looked like it was in the commits. Omega has separate logic to ensure the clearfix is added to the node and comment so no layouts should be affected.

Eric agreed we shouldn't try to change how Omega is handling the array so issue should be all resolved ^_^.

Status: Fixed » Closed (fixed)

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