#3652 closed defect (bug) (fixed)
bp_core_new_nav_default() doesn't really work
Reported by: | boonebgorges | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 1.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Core | Keywords: | has-patch dev-feedback |
Cc: |
Description
bp_core_new_nav_default() is supposed to let you set a default subnav for a nav item other than the one originally registered with the nav item.
It doesn't really work, because of this check: http://buddypress.trac.wordpress.org/browser/tags/1.5/bp-core/bp-core-buddybar.php#L116 The !$bp->current_action check will never return true, because bp_core_new_nav_item() will always set a current_action (http://buddypress.trac.wordpress.org/browser/tags/1.5/bp-core/bp-core-buddybar.php#L82)
Have a feeling this is going to be tricky to fix, because with $bp->current_action set, I don't think there's an easy way for BP to know the difference between, eg
http://example.com/members/boone/settings/
and
http://example.com/members/boone/settings/general/
Obviously, when you change the default, you only want to change the behavior of the former URL, not the latter.
Maybe when we manually set $bp->current_action in bp_core_new_nav_item(), we should set a flag in the global that says that we did so? Have to think about it. Really we should also think about content duplication issues here too; if /members/boone/settings/ shows General, then we shouldn't have another URL /members/boone/settings/general/. See #1741.
Attachments (1)
Change History (6)
#1
@
13 years ago
- Keywords has-patch dev-feedback added; needs-patch removed
- Severity changed from minor to normal
#2
@
13 years ago
This probably works as expected again, given the way that the canonical redirect has been reordered. Needs testing.
This is indeed tricky. I can't figure out a way to fix this while maintaining any semblance of backpat with previous uses of bp_core_new_nav_default(). In a nutshell, here's how the current system works:
So, in short, the redirection, etc happens *immediately after* the nav item is added, which leaves no obvious place to change the behavior. You need to have access to the nav item in order to change the default subnav (so you need to hop in after it's been added), but you need to alter it before the redirect happens, or you'll be too late to modify the nav default.
3652.01.patch is a proposed solution. Here's how it works:
So, it's not totally backward compatible - those who are currently using bp_core_new_nav_default() will have to change their priority so that it happens *before*, rather than *after*, the nav item is added. And it's not particularly elegant. But it works. The only other alternative I see is to take the redirecting stuff and to put it much later in the BP loading process - which is fine, except that it will make the redirects happen more slowly by introducing more processing before they occur, which is IMO a bad thing.
Would be happy to get some feedback on this issue. Thanks.