Was pocking around in template.php and noticed there's no default case for $vars['build_mode'], so seemed like a good idea to add that in. Also, adding it as a class seems useful also.



ceardach’s picture

Yeah, I actually ended up adding my own build mode variable in my custom theme, too. +1 adding this to zen.

bangpound’s picture

new1.23 KB

This is a good idea, but doesn't fix a small bug.

The problem with the 'switch' statement is that it uses loose comparison. I'm not going to start defining arbitrary integers for my build modes, so I'm going to use strings.

When build mode is a string, $node->build_mode == NODE_BUILD_NORMAL. Constants are ideally compared with ===, but there's no way to get a switch statement to do this.

This patch lets you keep the switch statement but it double checks that $node->build_mode === NODE_BUILD_NORMAL.

bangpound’s picture

This patch fixes conflicts between Zen theme and potentially a handful of other modules that implement hook_content_build_modes that use strings as build mode identifiers.

See: http://drupalcontrib.org/api/search/6/_content_build_modes

Can we please please please commit this patch?

hefox’s picture

If NODE_BUILD_NORMAL is put right above default:, and break moved into the if statement {}, it can fallback to default cause if not NODE_BUILD_NORMAL, which I believe is desirable, right?

mlncn’s picture

Status:Needs review» Reviewed & tested by the community

Tested, approved, and in use on production. Please commit!

JohnAlbin’s picture

Component:PHP Code» PHP code
Status:Reviewed & tested by the community» Fixed

I just committed http://drupalcode.org/project/zen.git/commit/d90f68f

But, looking at this again, doesn't the current code's if ($vars['node']->build_mode === NODE_BUILD_NORMAL) mean that $vars['build_mode'] won't be set if the build mode loosely matches NODE_BUILD_NORMAL but doesn't exactly match NODE_BUILD_NORMAL?

Don't we need this?

      if ($vars['node']->build_mode === NODE_BUILD_NORMAL) {
        $vars['build_mode'] = $vars['teaser'] ? 'teaser' : 'full';
      else {
        $vars['build_mode'] = $vars['node']->build_mode;

I committed the else clause as well. http://drupalcode.org/project/zen.git/commit/6a2a1ae

Please re-open if I'm off base.

hefox’s picture

Yep, the else statement was needed. Thanks :)

Status:Fixed» Closed (fixed)

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

JohnAlbin’s picture

Version:6.x-2.x-dev» 7.x-3.x-dev
Status:Closed (fixed)» Fixed

We need a corresponding view mode class in D7 as well. Just added it. :-)


Status:Fixed» Closed (fixed)

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