Edgewall Software
Modify

Opened 11 years ago

Last modified 19 months ago

#9528 new defect

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

Reported by: olemis+trac@… Owned by:
Priority: normal Milestone: next-stable-1.4.x
Component: web frontend Version: 0.12
Severity: minor Keywords: mainnav RepositoryManager
Cc: ethan.jucovy@…, ryan.j.ollos@…, fcorreia@…, leho@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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 .

Attachments (1)

t9528_r11480_ensure_lazy_chrome.diff (1.8 KB ) - added by olemis+trac@… 9 years ago.
Patch: Trac #9528 : Ensure lazy instantiation of req.chrome

Download all attachments as: .zip

Change History (18)

comment:1 by Christian Boos, 11 years ago

Resolution: cantfix
Status: newclosed

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

in reply to:  1 ; comment:2 by anonymous, 11 years ago

Resolution: cantfix
Status: closedreopened

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.

in reply to:  2 comment:3 by olemis+trac@…, 11 years ago

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 .

comment:4 by Christian Boos, 11 years ago

Component: version controlweb frontend
Milestone: next-minor-0.12.x
Severity: normalminor
Version: 0.12

Ok, seems worth having a look.

comment:5 by Ryan J Ollos <ryan.j.ollos@…>, 9 years ago

#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.

in reply to:  5 comment:6 by olemis+trac@…, 9 years ago

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 .

comment:7 by ethan.jucovy@…, 9 years ago

Cc: ethan.jucovy@… added

by olemis+trac@…, 9 years ago

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

comment:8 by olemis+trac@…, 9 years ago

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 .

comment:9 by olemis+trac@…, 9 years ago

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 ?

comment:10 by Ryan J Ollos <ryan.j.ollos@…>, 9 years ago

Cc: ryan.j.ollos@… added

comment:11 by fcorreia@…, 9 years ago

Cc: fcorreia@… added

comment:12 by lkraav <leho@…>, 9 years ago

Cc: leho@… added

comment:13 by Ryan J Ollos, 7 years ago

Milestone: next-minor-0.12.xnext-stable-1.0.x

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?

comment:14 by fcorreia@…, 7 years ago

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

comment:15 by Ryan J Ollos, 7 years ago

Status: reopenednew

comment:16 by Ryan J Ollos, 5 years ago

Milestone: next-stable-1.0.xnext-stable-1.2.x

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.

comment:17 by Ryan J Ollos, 19 months ago

Milestone: next-stable-1.2.xnext-stable-1.4.x

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.