Closed (fixed)
Project:
Pageroute
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
17 Aug 2009 at 11:31 UTC
Updated:
8 Sep 2009 at 10:10 UTC
Jump to comment: Most recent file
* I just tried the node-management pages, it doesn't respect show the forwared/back buttons and doesn't hide the other buttons, as configured.
* There are quite some php notices, be sure to develop with php notices enabled. There shouldn't be any.
* Minor, but in the UI I don't think it makes much sense to show the "delete route" callback as tab. Best just remove the tab, as there is still the button to delete.
Best also make sure the button settings are also working right for node add/edit pages and always try to cover such bugs by simpletests.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | routepage_bugtestrun1_003.diff | 9.52 KB | sepgil |
| #5 | routepage_bugtestrun1_002.diff | 9.56 KB | sepgil |
| #3 | routepage_bugtestrun1_001.diff | 9.56 KB | sepgil |
Comments
Comment #1
fagoah also the cancel link is missing
Comment #2
fagoQuestion: Should pageroutepage::nodeUI be protected?
Comment #3
sepgil commentedMade a patch that corrects all the issues, except the "Delete route" Tab. I assueme that you mean the tab while viewing all pages of a route on the admin site(admin/build/pageroute/[route_id]). I tried to remove it, but than it also deleted the button on the main pageroute admin site.
I'm not sure if I cleared all notices. I get a lot of Undefined offset notices from menu.inc, and I don't if they are produced by pageroute or by my local development setup.
Comment #4
fagoI also for the node management page I still get a bunch of notices, e.g on the edit subpage:
# notice: Undefined variable: page in drupal-6/sites/all/modules/pageroute/pageroute.module on line 638.
# notice: Trying to get property of non-object in drupal-6/sites/all/modules/pageroute/pageroute.module on line 638.
Anyway patch looks good, but I found a coding style issue:
+ if (isset($page->options['current_group']) && $page->options['current_group']!='') {
You have to put spaces around "!= ". Anyway for such checks the best way is to use !empty($variable) which works right and doesn't generate a notice even if it's not set.
Comment #5
sepgil commentedchanged $page->options['current_group']!=' to !empty($page->options['current_group'])
Since we will kick out the tab functionallity, I didn't correct the notices...
Comment #6
fagooh, yep it works right, however you still use isset(). When you use empty() you don't need to check isset() too. -> Best correct the checks to only use emty() or !empty() for cases like + if (!isset($info['useForm']) || !$info['useForm']) {.
Comment #7
sepgil commentedremoved the isset()
Comment #8
fagothanks, committed.