#10748 closed enhancement (fixed)
For optimal performance req.chrome should be evaluated only if necessary.
Reported by: | Olemis Lang | Owned by: | Olemis Lang |
---|---|---|---|
Priority: | normal | Component: | ThemeEnginePlugin |
Severity: | minor | Keywords: | chrome css script |
Cc: | peter@…, Gary Martin | Trac Release: | 1.0 |
Description
When the plugin is installed and active it's post_process_request() will be invoked for every request. AFAICT there is nothing in that code that would differentiate requests (API vs UI requests) so add_stylesheet() will be called for all of them which then calls add_link() that evaluates req.chrome.
mentioned in BH:comment:3:ticket:330
Attachments (0)
Change History (8)
comment:1 Changed 12 years ago by
Status: | new → assigned |
---|
comment:4 Changed 12 years ago by
Type: | defect → enhancement |
---|
comment:5 follow-up: 6 Changed 12 years ago by
Cc: | Gary Martin added |
---|
I cleaned up the reference in your commit message :)
One minor, almost irrelevant comment that I'd like to get your thoughts on. PEP8 states: Comparisons to singletons like None should always be done with is or is not, never the equality operators.
So I was thinking that would suggest the following change:
if (template, data) != (None, None):
->
if (template, data) is not (None, None)
(I wish individual bullet points on PEP8 had anchors so that we could link to them individually. The section headings have anchors, but even for those the browser doesn't seem to jump to the right location [at least in Chrome 23, try: http://www.python.org/dev/peps/pep-0008/#id37, it should go Programming Recommendations section]).
comment:6 Changed 12 years ago by
Replying to rjollos:
[...]
So I was thinking that would suggest the following change:
if (template, data) != (None, None):->
if (template, data) is not (None, None)
Bad idea , mainly because of this
>>> (None, None) is (None, None) False
[...]
comment:7 Changed 12 years ago by
Ah, right, because in the latter case it will be testing if the two references are the same. Thanks for the clarification.
comment:6 Changed 12 years ago by
(In [12534]) ThemeEnginePlugin: [regression] Theme struct will be None if no theme configured or set to 'default' - refs #10748 fixes #10800
comment:7 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
This is done. Please reopen if anything weird is detected .
comment:8 Changed 12 years ago by
(In [12540]) ThemeEnginePlugin: Let theme post-process requests while rendering error pages - after [12508] refs #10748 fixes #10809
(In [12508]) ThemeEnginePlugin : Do not access
req.chrome
while filtering requests iftemplate = data = None
( refs #10748 )