Default "bookmark" flag doesn't do anything out of the box

this is just because 'story' was renamed to 'article' in D7. Pretty simple patch.

But 'article' is missing if one is upgrading to D7 from D6 (I think).

So instead of having a fixed array of node types, we can do:

$flag_node_types = array_intersect(array('story', 'article', 'forum', 'blog'), array_keys(node_type_get_types()));
if (!$flag_node_types) {
  // 'page' too may not exist, but we want something to show on the admin page:
  $flag_node_types = array('page');
}

(For D6 we'll use the same code but replace "node_type_get_types" with "node_get_types".)

This also prevents a WTF moment when one sees "blog, forum" where these modules aren't installed.

Nate: before I write a patch, are you ok with this approach?

I didn't know Flag had its own tests. I bet those are busted, too.

We'll deal with the tests in a separate issue: #901136: Fix the existing tests so they pass.

I'm marking this 'critical'.

Nate, are you fine with the fix I suggest in #2 ?

(I know there is/was a discussion about making a flag apply to all node-types, if none is specified, and this idea indeed has its merits, but we need a solution for the nearer future.)

Nate, are you OK with this patch?

(For D6 we'll call node_get_types() instead of node_type_get_types(), and remove "article" from the $preferred.)

This sounds good to me, but it seems unnecessary to make a separate function for getting the list of types. I assume this is so that D6 and D7 have similar hook_enable() functions? Looks good enough to me in any case.

Committed.
http://drupal.org/cvs?commit=448314
http://drupal.org/cvs?commit=448310

but it seems unnecessary to make a separate function for getting the list of types. I assume this is so that D6 and D7 have similar hook_enable() functions?

Sort of. I didn't plan to do this in a separate function from the start, but PHP isn't a compact language and after some thinking I decided it looked less awful in a separate func.