#15368 closed (fixed)
Regression in Context
Reported by: | Timothée Peignier | Owned by: | Luke Plant |
---|---|---|---|
Component: | Template system | Version: | 1.2 |
Severity: | Keywords: | blocker regression | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
There is a regression since r15601 (in the 1.2.X branch), that notably break tests :
def _new(self): return self.__class__(autoescape=self.autoescape, current_app=self.current_app, use_l10n=self.use_l10n)
But the use_l10n
attribute doesn't exist in 1.2.
Attachments (4)
Change History (21)
comment:1 by , 14 years ago
Keywords: | blocker regression added |
---|---|
milestone: | → 1.3 |
Triage Stage: | Unreviewed → Ready for checkin |
by , 14 years ago
Attachment: | 15368-regression-context.diff added |
---|
comment:2 by , 14 years ago
comment:3 by , 14 years ago
Owner: | changed from | to
---|---|
Triage Stage: | Ready for checkin → Accepted |
Ok - moving back to accepted. Also putting it on SmileyChris' radar, since he broke it :-)
comment:5 by , 14 years ago
Actually, I've fixed the use_l10n part of this in r15618.
I'll leave this ticket open to track the issue of copying the context. The issue is that we need to make a new instance of RequestContext
, but don't have any reference to request
at that time so I faked it with a bare HttpRequest
(and the problems around this is what I was saying I thought about earlier).
I'm thinking that perhaps I could change RequestContext
to allow for a request of None
which simply skips the running of the context processors during instantiation.
by , 14 years ago
Attachment: | 15368-requestcontext.diff added |
---|
comment:6 by , 14 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Ok, put up a patch but needs tests still.
comment:7 by , 14 years ago
@SmileyChris - why do we need to use the class constructor to copy the instance? Could we not just implement __copy__
differently on RequestContext
? In fact, do we need to implement __copy__()
at all? You can simplify things quite a bit by implementing a copy()
method which just uses copy.copy()
with a few additions, and then using that copy method rather than copy.copy()
on the Context object.
I'll attached some tests for the regression involving RequestContext
, and my proposed fix.
follow-up: 9 comment:8 by , 14 years ago
luke: elsewhere in core the __copy__
method had been used, so I was just following precidence. I'm also fine with an explicit .copy()
or .clone()
method.
comment:9 by , 14 years ago
Replying to SmileyChris:
luke: elsewhere in core the
__copy__
method had been used, so I was just following precidence. I'm also fine with an explicit.copy()
or.clone()
method.
Sure, I think it just happens that in this case it is simpler not to implement __copy__()
.
However, there is a way to implement __copy__()
and still avoid the need for calling constructors - like this:
class NoInitArgs(object): pass class MyClass(object): def __copy__(self): duplicate = NoInitArgs() duplicate.__class__ = self.__class__ duplicate.__dict__ = self.__dict__.copy() return duplicate
comment:10 by , 14 years ago
Owner: | changed from | to
---|
Taking ownership, since we didn't hear from Chris for a few days, and I'm aware of the earthquake in New Zealand (hope everything is OK with you and family and friends, Chris!).
comment:13 by , 14 years ago
Sorry, should have mentioned here that I had a branch ready for merging :P
https://github.com/SmileyChris/django/tree/15368-copy-requestcontext
I'll apply the 1.3 notes today some time.
Note that in my branch, I just used copy(super(...)) which works rather than requiring a placeholder object.
comment:14 by , 14 years ago
Sorry, Chris, didn't mean to be impatient, but with this blocking the RC we were supposed to release this week, and with it affecting my own projects, I went ahead and committed a fix. Feel free to improve what I did!
I've added a patch, that actually only scratch the surface, I have a new error when trying to access request.user in a context_processor in my tests :
I will try to get a regression test working.