Failed to highlight active mainnav item if `RepositoryManager` is enabled

[olemis+trac@…]

I am experiencing an odd behavior using Trac and ​ThemeEnginePlugin . The problem is that active CSS class is not added to active mainnav item. Everything works ok if I disable RepositoryManager . I've tried to find the exact location where the problem happens , but without success . The only things I can say so far are :

• handler var is set to None when calling Chrome.prepare_data …
• … therefore active var is also set to None , and item is not highlighted in mainnav …
• … the strange fact is that at that time , the partial object has already been installed for chosen_handler inside RequestDispatcher.dispach since req.callbacks['chrome'].keywords.items() contains the correct binding for handler key.
• the signature of RepositoryManager.post_process_request is post_process_request(self, req, template, content_type) , is it suitable to be used with Genshi templates ?

PS: Posted here 'cause I think there's a problem with Trac rather than the plugin .

[Christian Boos]

Is there a problem without the theme engine plugin? If yes, please reopen. Otherwise I'm afraid you're on your own.

[anonymous]

Replying to cboos:

Is there a problem without the theme engine plugin? If yes, please reopen. Otherwise I'm afraid you're on your own.

The problem is that if a filter instantiates req.chrome inside pre_process_request then process_data is not wrapped using functools.partial, hence it doesn't receive active handler , and finally there's no mainnav highlighted.

[olemis+trac@…]

Replying to anonymous:

Replying to cboos:

Is there a problem without the theme engine plugin? If yes, please reopen. Otherwise I'm afraid you're on your own.

The problem is that if a filter instantiates req.chrome inside pre_process_request then process_data is not wrapped using functools.partial, hence it doesn't receive active handler , and finally there's no mainnav highlighted.

… even if req.callbacks['chrome'].keywords.items() contains the correct binding for handler key , because that setup happened after instantiating chrome attribute .

[Christian Boos]

Ok, seems worth having a look.

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

#10953 might be a duplicate. The issue that I think I'm seeing in that ticket is that the add_warning in RepositoryManager.pre_process_request causes the prepare_request callback to execute immediately with chosen_handler set to None.

[olemis+trac@…]

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

#10953 might be a duplicate.

definitely yes . Ethan has submitted a patch​ explained here .

The issue that I think I'm seeing in that ticket is that the add_warning in RepositoryManager.pre_process_request causes the prepare_request callback to execute immediately with chosen_handler set to None.

definitely right .

[olemis+trac@…]

Patch: Trac #9528 : Ensure lazy instantiation of req.chrome

[olemis+trac@…]

The only objection I have after reviewing Ethan's patch​ is that the side effects of its last hunk will imply that instantiation of req.chrome would never be lazy any more. Therefore I propose to apply attached patch​ on top of Ethan's . I've been doing such thing with the help of a local mercurial patch queue . Patch order is as follows :

$ hg qapplied
trac/t9528/t10953-r11480-no-handler-in-chrome-prepare_request.diff
trac/t9528/t9528_r11480_ensure_lazy_chrome.diff

Please review .

[olemis+trac@…]

I've been taking a look once again and it seems to me that there are another issues with Ethan's patch :

1. nav items won't be available if request dispatching never gets to the point where prepare_request_with_handler is invoked , included but not limited to
   • error raised in _pre_process_request
   • no handler found
2. extended version of req.chrome won't be available while invoking chosen_handler.process_request ?

[Ryan J Ollos]

Moving this forward since I don't think it needs to be fixed on 0.12-stable, but please comment if you feel differently. Is anyone interested in providing a revised patch?

[fcorreia@…]

This is very annoying, I think it would be worth to have it for the next 0.12. Is the current patch now outdated?

[Ryan J Ollos]

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

[Ryan J Ollos]

Milestone renamed