Add default case for build_mode [#774838]

Comment	File	Size	Author
#2	build_mode_class-774838-2.patch	1.23 KB	Anonymous (not verified)
	build_mode_class.patch	603 bytes	hefox

Comment #1

ceardach CreditAttribution: ceardach commented 18 April 2010 at 22:18

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

Log in or register to post comments

Comment #2

Anonymous (not verified) CreditAttribution: Anonymous commented 19 August 2010 at 16:34

File	Size
build_mode_class-774838-2.patch	1.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.

Log in or register to post comments

Comment #3

Anonymous (not verified) CreditAttribution: Anonymous commented 1 January 2011 at 22:18

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?

Log in or register to post comments

Comment #4

hefox CreditAttribution: hefox commented 2 January 2011 at 01:44

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?

Log in or register to post comments

Comment #5

mlncn CreditAttribution: mlncn commented 7 January 2011 at 21:20

Status:

Needs review

» Reviewed & tested by the community

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

Log in or register to post comments

Comment #6

JohnAlbin

he/they/他

English

Taipei, Taiwan

CreditAttribution: JohnAlbin commented 4 April 2011 at 19:58

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?

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

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

Please re-open if I'm off base.