Documentation says that 0x0010 is what makes MENU_CALLBACK different from MENU_SUGGESTED_ITEM when it is not true

[kiamlaluno]

The documentation for MENU_SUGGESTED_ITEM contains the following sentence.

Note for the value: 0x0010 was a flag which is no longer used, but this way the values of MENU_CALLBACK and MENU_SUGGESTED_ITEM are separate.

The highlighted part is not true: MENU_SUGGESTED_ITEM is MENU_VISIBLE_IN_BREADCRUMB | 0x0010 (0x0014) while MENU_CALLBACK is 0x0000; even removing 0x0010, those constants would have two different values (0x0004 and Ox0000).

Truly, 0x0010 is what makes MENU_SUGGESTED_ITEM different from MENU_VISIBLE_IN_BREADCRUMB. That sentence should be the following one.

Note for the value: 0x0010 was a flag which is no longer used, but this way the values of MENU_VISIBLE_IN_BREADCRUMB and MENU_SUGGESTED_ITEM are separate.

[stpaultim]

I'm not entirely able to confirm that this makes sense, but I kinda trust that it does. If so, easy and simple change. I have this a Milestone Candidate - Bug tag to hopefully increase awareness. Does anyone want to add this to the bugfix milestone, so that it gets attention. No reason to leave a PR this this simple hanging for months or years. Either it's a good idea or it isn't.

[kiamlaluno]

@stpaultim I think this is an internal note that reminds that 0x0010 in MENU_SUGGESTED_ITEM is what makes that constant different from MENU_VISIBLE_IN_BREADCRUMB, so 0x0010 cannot be removed from the definition of MENU_SUGGESTED_ITEM. (The current note is not correct and could make people think that 0x0010 can be removed from the definition of MENU_SUGGESTED_ITEM.)

[jenlampton]

@stpaultim to be fair, the original sentence wasn't very clear either. This might be a good time to improve that also :)

How about something like:

Note: The value 0x0010 cannot be removed from the definition of MENU_SUGGESTED_ITEM. It is a flag (no longer used) that at one time ensured that the values of MENU_CALLBACK and MENU_SUGGESTED_ITEM were separate.

We should probanly also look at the issue where MENU_VISIBLE_IN_BREADCRUMB was added. We might want to add something about that to explain why it's MENU_VISIBLE_IN_BREADCRUMB now when in the past it was MENU_CALLBACK.

[kiamlaluno]

Drupal 5.x has the following definitions.

define('MENU_VISIBLE_IN_BREADCRUMB', 0x0004);
define('MENU_MODIFIABLE_BY_ADMIN', 0x0010);
define('MENU_CALLBACK', MENU_VISIBLE_IN_BREADCRUMB);
define('MENU_SUGGESTED_ITEM', MENU_MODIFIABLE_BY_ADMIN | MENU_VISIBLE_IN_BREADCRUMB);

Drupal 6.x has these definitions.

define('MENU_VISIBLE_IN_BREADCRUMB', 0x0004);
define('MENU_CALLBACK', MENU_VISIBLE_IN_BREADCRUMB);
define('MENU_SUGGESTED_ITEM', MENU_VISIBLE_IN_BREADCRUMB | 0x0010);

Drupal 7.x has these definitions.

define('MENU_VISIBLE_IN_BREADCRUMB', 0x0004);
define('MENU_CALLBACK', 0x0000);
define('MENU_SUGGESTED_ITEM', MENU_VISIBLE_IN_BREADCRUMB | 0x0010);

The comment that is currently used in Backdrop (which is the following one) was true for Drupal 5 and Drupal 6, but not Drupal 7, for which the value of MENU_CALLBACK changed from MENU_VISIBLE_IN_BREADCRUMB to 0x0000.

Note for the value: 0x0010 was a flag which is no longer used, but this way the values of MENU_CALLBACK and MENU_SUGGESTED_ITEM are separate.

Using git blame I found this commit, but it does not seem it removed MENU_MODIFIABLE_BY_ADMIN. I could not say why or when that constant was removed.

MENU_VISIBLE_IN_BREADCRUMB has been defined since Drupal 4.5. (I cannot get a commit link for the commit that introduced it.)

[kiamlaluno]

I changed the note as suggested.

[klonos]

LGTM 👍🏼 ...let's try to knock this one out of the way 🙂