Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Looking through Menu Block while sorting out some "active-trail" issues, I came across this code
if (!empty($menu_item['in_active_trail'])) {
if (!empty($menu_item['localized_options']['attributes']['class'])) {
$menu_item['localized_options']['attributes']['class'][] = 'active-trail';
}
else {
$menu_item['localized_options']['attributes']['class'][] = 'active-trail';
}
}
You'll notice that the code is identical inside the if and inside the else, so the test is doing nothing. I assume this is some legacy code, but it could be that something else was intended. So I'm attaching a patch that cleans up the code, but the maintainers will know whether there's a logic problem that needs fixing.
if (!empty($menu_item['in_active_trail'])) {
$menu_item['localized_options']['attributes']['class'][] = 'active-trail';
}
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 2194115-simplify-class-setting.patch | 764 bytes | Dave Reid |
| menu_block-7.x-3x-remove-extra-conditional.patch | 762 bytes | ergophobe | |
Comments
Comment #1
ergophobe CreditAttribution: ergophobe commentedPS, this is based on a git pull of a few minutes ago and there is no diff between 7.x-3.x and 7.x-2.x, so despite my file name, it should apply equally to either branch.
Comment #2
othermachines CreditAttribution: othermachines commentedThat statement is checking whether the variable exists, so the change should probably be
Comment #3
Dave ReidActually we can just set it normally, because $menu_item['localized_options'] is always an array: http://3v4l.org/CMSU6
Comment #5
Dave ReidComment #9
Dave ReidCommitted #3 to 7.x-2.x.
Comment #11
ergophobe CreditAttribution: ergophobe commentedThanks Dave for cleaning and committing.
I don't really care about credit on a patch like this, but I've noticed that even though committers often go to the trouble to add me in the commit message, it rarely shows up as one of my commits in my profile. Do you know why that is?
[edit - I think I know why - to make it easy for committers to give you credit, you should use git format-patch but I always just use git diff > filename.patch. As per https://www.drupal.org/node/1146430
Comment #12
Dave ReidBecause there were more than two people involved with submitting patchs, I don't usually attribute commits (but still mention in the commit message). If I had been able to use your patch as-is, I do always use attribution.
Comment #13
Dave ReidHrm, I now see that our patches were pretty much identical. It needed a re-roll since it no longer applied, but I probably *should* have attributed it to you. My apologies about that.
Comment #14
ergophobe CreditAttribution: ergophobe commentedDave, my patch was so simple and represented no real work. I was not at all miffed that I didnt' get credited.
I have done some patches that represented fairly significant work and don't get credit and others like this that basically fix a typo and I have sometimes gotten credit. I was just trying to understand why.
I certainly did not mean any reproach - you do so much for the community already that I hesitated to even mention it here, but was just curious. Between your answer and the link I had above I have a better understanding.
Thanks for the response and thanks, belatedly for the useful modules and zillion patches you've done. It is deeply appreciated.