Active mainnav navigation entry is not highlighted when running tracd

[Ryan J Ollos <ryan.j.ollos@…>]

When running tracd in a development environment with the latest version of the trunk, the active navigation item is never highlighted. I've tried debugging the issue. The active navigation item appears to be determined here. If I print handler and contributor at those lines:

handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.web.auth.LoginModule object at 0xa76890c>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.about.AboutModule object at 0xa768e4c>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.attachment.AttachmentModule object at 0xa7688ec>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.admin.web_ui.AdminModule object at 0xa76894c>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.prefs.web_ui.PreferencesModule object at 0xa7688cc>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.search.web_ui.SearchModule object at 0xa768aac>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.ticket.query.QueryModule object at 0xa7684ac>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.ticket.report.ReportModule object at 0xa7684cc>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.ticket.roadmap.RoadmapModule object at 0xa768acc>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.ticket.roadmap.MilestoneModule object at 0xa7685ac>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.ticket.web_ui.TicketModule object at 0xa76858c>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.timeline.web_ui.TimelineModule object at 0xa76850c>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.versioncontrol.web_ui.browser.BrowserModule object at 0xa768ecc>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.versioncontrol.web_ui.changeset.ChangesetModule object at 0xa768eec>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.versioncontrol.web_ui.log.LogModule object at 0xa76856c>
handler     = <trac.web.chrome.Chrome object at 0xa76d92c>
contributor = <trac.wiki.web_ui.WikiModule object at 0xa768d0c>

I don't understand the architecture enough to determine what might be wrong here.

[Ethan Jucovy <ethan.jucovy@…>]

Which navigation menu do you mean — mainnav, metanav, or ctxtnav?

On trunk@r11479 I'm seeing the active mainnav entry highlighted correctly when running tracd /path/to/testenv -p 8000.

The active metanav entry (e.g. if I GET /prefs/) is correctly being rendered as an <li class="active"> but there's no CSS being applied to that entry.

The active ctxtnav entry (e.g. if I GET /wiki/TitleIndex/) is not being rendered as an <li class="active"> at all.

For debugging, probably the best thing to do would be to pprint( chrome['nav'] ) at line 747 (right after chrome['nav'] is set)

[Ryan J Ollos <ryan.j.ollos@…>]

Replying to Ethan Jucovy <ethan.jucovy@…>:

Which navigation menu do you mean — mainnav, metanav, or ctxtnav?

mainnav

On trunk@r11479 I'm seeing the active mainnav entry highlighted correctly when running tracd /path/to/testenv -p 8000.

Just tested again with r11479 and starting tracd in the same way, and I still see the issue.

[…] For debugging, probably the best thing to do would be to pprint( chrome['nav'] ) at line 747 (right after chrome['nav'] is set)

Here is what I get:

{'mainnav': [{'active': False, 'label': <Element "a">, 'name': 'wiki'},
             {'active': False, 'label': <Element "a">, 'name': 'timeline'},
             {'active': False, 'label': <Element "a">, 'name': 'roadmap'},
             {'active': False, 'label': <Element "a">, 'name': 'tickets'},
             {'active': False, 'label': <Element "a">, 'name': 'newticket'},
             {'active': False, 'label': <Element "a">, 'name': 'search'},
             {'active': False, 'label': <Element "a">, 'name': 'admin'}],
 'metanav': [{'active': False, 'label': <Element "a">, 'name': 'login'},
             {'active': False, 'label': <Element "a">, 'name': 'prefs'},
             {'active': False, 'label': <Element "a">, 'name': 'help'},
             {'active': False, 'label': <Element "a">, 'name': 'about'}]}

The result is the same regardless of which mainnav entry I select.

[Ethan Jucovy <ethan.jucovy@…>]

Replying to Ryan J Ollos <ryan.j.ollos@…>:

The result is the same regardless of which mainnav entry I select.

It's strange that you're seeing handler = <trac.web.chrome.Chrome object at 0xa76d92c> — from my testing, that should only be the case when you're visiting a URL like /chrome/site/your_project_logo.png. When visiting /wiki/WikiStart for example my tracd server reports that hander = <trac.wiki.web_ui.WikiModule object at 0x1081eded0>.

That incorrect handler seems to be the problem: the nav bar will only register an active entry if the current IRequestHandler is an INavigationContributor component.

Maybe print req, req.abs_href(), handler as well, in the same place in the code?

Do you have anything nonstandard in [components]?

[Ryan J Ollos <ryan.j.ollos@…>]

Replying to Ethan Jucovy <ethan.jucovy@…>:

It's strange that you're seeing handler = <trac.web.chrome.Chrome object at 0xa76d92c> — from my testing, that should only be the case when you're visiting a URL like /chrome/site/your_project_logo.png. When visiting /wiki/WikiStart for example my tracd server reports that hander = <trac.wiki.web_ui.WikiModule object at 0x1081eded0>.

I may have been posting some misleading debug information. There seems to be a handler for the request, followed by a handler for chrome. I can somewhat reconcile this with what I see in TracDev/RequestHandling.

That incorrect handler seems to be the problem: the nav bar will only register an active entry if the current IRequestHandler is an INavigationContributor component.

Maybe print req, req.abs_href(), handler as well, in the same place in the code?

Do you have anything nonstandard in [components]?

Nothing nonstandard in components, just a clean Trac instance that is created with a shell script to setup a development environment.

I've found a critical element of reproducing this issue. The unsupported version control system warning was present due to not having a version control component installed as well as not having the bindings installed. Once I install the SVN bindings and enable the SVN component, the warning message goes away and the main nav items are correctly highlighted.

What I'm seeing is that the call to add_warning seems to trigger the prepare_request callback, and the chosen_handler argument is None because the callback doesn't get set with a chosen_handler until ​here. If I comment out add_warning in RepositoryManager.pre_process_request the active navigation item is correctly highlighted.

[Ryan J Ollos <ryan.j.ollos@…>]

This might be a duplicate of #9528, but some of the details in that issue differ slightly.

[Ryan J Ollos <ryan.j.ollos@…>]

The following t10953-r11479-1.patch​ is more of a workaround than a solution, but it fixes the issue.

[Ethan Jucovy <ethan.jucovy@…>]

This is definitely a duplicate of #9528. A few more details:

• The failure occurs whenever add_warning is called in an IRequestFilter.pre_process_request method. After pre_process_request, it's fine to call add_warning.
• Chrome.prepare_request is called because add_warning accesses req.chrome which is a lazily-evaluated attribute that calls Chrome.prepare_request the first time it's accessed, and then stores that function's return value on the request.
• This bug will recur in any request where req.chrome is accessed in an IRequestFilter.pre_process_request method.
• The req.chrome callback is initially set up (in trac.web.main.RequestDispatcher.dispatch) to simply call Chrome.prepare_request(req). Later in the same function, after the active request handler has been identified, the callback is updated (using functools.partial as mentioned in the other ticket) to call `Chrome.prepare_request(req, active_handler)
• However, the IRequestFilter.pre_process_request methods are called before the callback has been updated, so chrome.prepare_request is executed without an active handler
• The callback can't be updated to include the active handler before IRequestFilter.pre_process_request is called — because IRequestFilter.pre_process_request is allowed to change the active handler.

So I think the only way to solve this "properly" and thoroughly would be to ensure that none of the code in chrome.prepare_request relies on the active handler having been set already.

[Ethan Jucovy <ethan.jucovy@…>]

Split out all handler-dependent code from chrome.prepare_request

[Ethan Jucovy <ethan.jucovy@…>]

So I think the only way to solve this "properly" and thoroughly would be to ensure that none of the code in chrome.prepare_request relies on the active handler having been set already.

I've attached a patch attachment:t10953-r11480-no-handler-in-chrome-prepare_request.diff​​ that implements this, and would solve the problem described here and in #9528. This patch contains the following changes:

• All code in Chrome.prepare_request whose behavior differs based on the handler argument has been removed from that method. This includes just two things: building the contents of chrome['nav'], and adding the jquery_noconflict script if the active handler is requesting it.
• Those two handler-dependent code blocks have been moved to a separate method, Chrome.prepare_request_with_handler. This method must be called with a non-None handler, which is expected to be the "final" handler chosen for the current request. This method returns a dict of additional chrome items to update the chrome dict with.
• The Chrome.prepare_request method signature no longer contains the optional handler=None argument. Since any handler-dependent code should be executed in prepare_request_with_handler, the signature of Chrome.prepare_request is now just (self, req) with no handler.
• Likewise, the req.callbacks['chrome'] callback is now just chrome.prepare_request at all times; it is not updated to be a functools.partialthat curries the chosen handler.
• The Chrome.prepare_request_with_handler method is called directly from RequestDispatcher.dispatch_request immediately after the chosen handler has processed the request, and before any post_process_request methods are called and before Chrome.render_template is called.

As far as I can tell Chrome.prepare_request is not really intended to be a public API method, so if that's true then changing the signature shouldn't be a big problem.

The side effect of this change is that req.chrome['nav'] is not available during IRequestHandler.process_request — it only becomes available for IRequestFilter.post_process_request methods. If this is a non-starter, it might be possible to set chrome['nav'] to be a lazily evaluated function itself, but that seems like it would be trickier, and a more explicit approach would be preferable if at all possible.

I haven't run the Trac tests yet — they would definitely need to be updated for this patch.

[olemis+trac@…]

Actually this ticket is a duplicate of #9528 , already scheduled for next-minor-0.12.x by cboos . I invite you to follow discussion in there . I'll reply with further details soon .

[Ryan J Ollos <ryan.j.ollos@…>]

Yes, that was already mentioned. However, this ticket has a patch now by Ethan, #9528 does not.

[olemis+trac@…]

Replying to olemis+trac@…:

I invite you to follow discussion in there . I'll reply with further details soon .

Needless to say , feel free to reopen this ticket if you think closing it as a duplicate is not the right way to go . However I see no reason for having two tickets for the same issue . We can also close that one keep this one open …

[Ryan J Ollos <ryan.j.ollos@…>]

I was just worried about losing the detailed explanation in this ticket. Since you've linked to the patch, I think it is great to continue in #9528, and we can pull over any details as necessary.