Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#8021 closed defect (wontfix)

calling add_ctxtnav from IRequestFilter.pre_process_request breaks active flag in mainnav

Reported by: jeremie.allard@… Owned by: rblank
Priority: normal Milestone:
Component: rendering Version: 0.11.2.1
Severity: normal Keywords:
Cc:
Release Notes:
API Changes:

Description

Adding a local navigation item can be done by a plugin by implementing IRequestFilter and calling add_ctxtnav within pre_process_request, such as in wiki:SubversionLocationPlugin. However this causes the active flag to disappear from the main navigation bar. See for example https://weblion.psu.edu/trac/weblion/browser

To reproduce it, the easiest way is to install wiki:SubversionLocationPlugin or to do a trivial plugin that just implement IRequestFilter and call add_ctxtnav(req, 'TEST') within pre_process_request.

I think this is because the chrome property is not yet ready within the request when calling pre_process_request.

I make a small patch that fix this, but as I'm not a python expert and this involves lazy evaluation of the chrome property I'm not sure if it is right.

Attachments (1)

requestfilter_add_ctxtnav_active.patch (1.3 KB) - added by Jeremie Allard <nospam@…> 5 years ago.
patch setting the handler in req.chrome before calling pre_process_request

Download all attachments as: .zip

Change History (6)

Changed 5 years ago by Jeremie Allard <nospam@…>

patch setting the handler in req.chrome before calling pre_process_request

comment:1 Changed 5 years ago by rblank

  • Milestone set to 0.11.3
  • Owner set to rblank

I'll look into this. Thanks for reporting the issue.

comment:2 follow-up: Changed 5 years ago by osimons

I think this is a wontfix personally, as reading the API docs for pre_process_request() and post_process_request() shows that they have different purposes. Here is the docs for pre_:

"Called after initial handler selection, and can be used to change the selected handler or redirect request."

At this stage your filter cannot know anything about the request, and it may well not be a request involving template rendering at all. By design, there will always be a number of things not yet initalised at the pre stage.

Use post_process_request().

comment:3 in reply to: ↑ 2 Changed 5 years ago by cboos

  • Milestone 0.11.3 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Agreed, closing as wontfix.

comment:4 Changed 5 years ago by Jeremie Allard <nospam@…>

I understand and agree with your analysis. As I did not know how Genshi worked, I believed that by the time post_process_request() was called, the html page was already generated and it was too late to call add_ctxtnav(). But this is not the case, so it is indeed the right way to do it. I'll submit a patch to the plugin maintainer…

comment:5 Changed 5 years ago by erose

And I've applied your patch on trunk. Thanks! TracSubversionLocation? 1.0.1 should be out momentarily.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed The owner will remain rblank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from rblank to the specified user.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.