Full error message:
Notice: Undefined offset: 3 in _menu_translate() (line 777 of /home/martin/www/purple/includes/menu.inc).
Happening on the path flickr/%uid/tags. A live example: https://www.visualeffectssociety.com/flickr/10606/tags
This is likely due to the menu implementation of the pretty old code base of the Flickr Tags sub-module.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | notice_undefined-2497799-15.patch | 1.27 KB | sumitmadan |
| #11 | tag_tabs-2497799-10.patch | 1.57 KB | lolandese |
| #9 | Flickr photos of admin tagged kids Demo of the Flickr module.jpeg | 41.79 KB | lolandese |
| #8 | notice_undefined-2497799-8.patch | 1.25 KB | sumitmadan |
| #2 | notice_undefined-2497799-2.patch | 1.83 KB | sumitmadan |
Comments
Comment #1
lolandese commentedComment #2
sumitmadan commentedPlease review. Also I would love to be a co-maintainer of the module.
Comment #3
lolandese commentedPatch applies cleanly, avoids the error (after cache clear) and improves the menu.
I really like what I see of your work on your approved project application and am happy to add you as co-maintainer. You should now be able to push the commit yourself.
Thanks.
Comment #5
sumitmadan commentedThanks lolandese,
I have committed the code to 7.x-1.x branch. :)
Comment #6
lolandese commentedOne small minor is the fact that the old way gives the possibility to get back to the tags cloud/list. See http://lolandese.altervista.org/flickr/1/tags/kids. We don't have this possibility anymore after the patch.
See if you can think of something to solve that. We can live without it. Most users know about the browser's 'back' button.
BTW while testing we found another unrelated issue, #2498821: Wrong images for tag list links.
Comment #7
sumitmadan commentedChecking the issue and about other issue I have commented on that. Please check that and I will commit both the fixes together.
Comment #8
sumitmadan commentedPlease review. You need to clear drupal cache because implemented a hook.
Comment #9
lolandese commentedAlmost. We would want:
where the third tab is visible and active only when a specific tag is shown.
Meanwhile the #8 patch can be committed as it improves the current situation already. Remember that commit messages will be used in drush generated release notes. Use something like
git commit -m 'Flickr Tags: Added tabs on tags page'and not the issue title if it doesn't cover for the change. Note how we use the sub-module name if it doesn't concern the main module.Thanks.
Comment #10
sumitmadan commentedActually, I have done this intentionally. Because a third tab seems weird. IMHO it should be like this only. But Please move status to needs work if we still need to add third tab. Otherwise I will commit this as final after your confirmation.
Comment #11
lolandese commentedI am sure it can be even more simplified now.
Is there a maybe a specific reason I am unaware of for using the referer instead of the current url?
Comment #12
sumitmadan commentedI used referer just to know the previous url of site. If it is cloud then the cloud tab will be active otherwise list tab will be active.
'#active' => ($url_base_name === 'cloud'),Basename can be "tags" for this because we are using "flickr/%user/tags" and "flickr/%users/tags/cloud" for same page. Or anyone can open the direct URL too by pasting it in browser's address bar. Thats why I used "$base_name != 'list'".
Comment #14
sumitmadan commentedIf we are keeping the third tab then no need to use #active on other tabs.
Also please change following code :
'#active' => ($url_base_name !== 'cloud' && $url_base_name !== 'list'),to :
'#active' => TRUE,Because a condition is already added before creating tab.
if ($root_path == 'flickr/%/tag/%') {Comment #15
sumitmadan commentedPlease have a look. :)
Comment #16
lolandese commentedWell done.
Remember what I wrote in #9, to use a commit message that makes sense in the automatically generated release notes.
Thanks.
Comment #18
sumitmadan commented